Page MenuHomeSoftware Heritage

api: Add endpoint to create an add-forge request
ClosedPublic

Authored by anlambert on Mar 8 2022, 3:29 PM.

Diff Detail

Repository
rDWAPPS Web applications
Branch
sprint-add-forge-now
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 27368
Build 42821: Phabricator diff pipeline on jenkinsJenkins console · Jenkins
Build 42820: arc lint + arc unit

Event Timeline

Build is green

Patch application report for D7311 (id=26448)

Could not rebase; Attempt merge onto 7e5b06ff77...

Merge made by the 'recursive' strategy.
 swh/web/add_forge_now/__init__.py                |   0
 swh/web/add_forge_now/apps.py                    |   5 ++
 swh/web/add_forge_now/migrations/0001_initial.py | 109 +++++++++++++++++++++++
 swh/web/add_forge_now/migrations/__init__.py     |   0
 swh/web/add_forge_now/models.py                  |  71 +++++++++++++++
 swh/web/api/urls.py                              |   1 +
 swh/web/api/views/add_forge_now.py               |  95 ++++++++++++++++++++
 swh/web/settings/common.py                       |   1 +
 swh/web/tests/api/views/test_add_forge_now.py    | 100 +++++++++++++++++++++
 swh/web/tests/utils.py                           |   4 +-
 10 files changed, 385 insertions(+), 1 deletion(-)
 create mode 100644 swh/web/add_forge_now/__init__.py
 create mode 100644 swh/web/add_forge_now/apps.py
 create mode 100644 swh/web/add_forge_now/migrations/0001_initial.py
 create mode 100644 swh/web/add_forge_now/migrations/__init__.py
 create mode 100644 swh/web/add_forge_now/models.py
 create mode 100644 swh/web/api/views/add_forge_now.py
 create mode 100644 swh/web/tests/api/views/test_add_forge_now.py
Changes applied before test
commit b3b72af2c07ac329c762b03a410deb93a626c0a6
Merge: 7e5b06ff b93e23c1
Author: Jenkins user <jenkins@localhost>
Date:   Tue Mar 8 14:29:51 2022 +0000

    Merge branch 'diff-target' into HEAD

commit b93e23c1f378239564fd940bab409ee1e7a3910c
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Tue Mar 8 15:23:50 2022 +0100

    api: Add endpoint to create an add-forge request
    
    Related to T3990

commit c5c3a06143dafb42a986540f25c0d333b49a7a1a
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Tue Mar 8 13:20:34 2022 +0100

    add_forge_now: Bootstrap app and model
    
    Summary: Related to T4019
    
    Test Plan: tox happy]
    
    Reviewers: #reviewers
    
    Maniphest Tasks: T4019
    
    Differential Revision: https://forge.softwareheritage.org/D7309

See https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1422/ for more details.

ardumont added a subscriber: ardumont.

lgtm

swh/web/tests/api/views/test_add_forge_now.py
55

use the enum

This revision is now accepted and ready to land.Mar 8 2022, 6:13 PM
vlorentz added inline comments.
swh/web/tests/api/views/test_add_forge_now.py
55

It's a test, it shouldn't depend on the actual code to be correct.

swh/web/tests/api/views/test_add_forge_now.py
55

I think i see what you mean... still, that's not entirely possible ;)
(anyway, fine)

Build is green

Patch application report for D7311 (id=26482)

Could not rebase; Attempt merge onto 7e5b06ff77...

Updating 7e5b06ff..d83389b9
Fast-forward
 swh/web/add_forge_now/__init__.py                |   0
 swh/web/add_forge_now/apps.py                    |  10 +++
 swh/web/add_forge_now/migrations/0001_initial.py | 109 +++++++++++++++++++++++
 swh/web/add_forge_now/migrations/__init__.py     |   0
 swh/web/add_forge_now/models.py                  |  71 +++++++++++++++
 swh/web/add_forge_now/tests/test_migration.py    |  62 +++++++++++++
 swh/web/api/urls.py                              |   1 +
 swh/web/api/views/add_forge_now.py               |  95 ++++++++++++++++++++
 swh/web/settings/common.py                       |   1 +
 swh/web/tests/api/views/test_add_forge_now.py    | 100 +++++++++++++++++++++
 swh/web/tests/utils.py                           |   4 +-
 11 files changed, 452 insertions(+), 1 deletion(-)
 create mode 100644 swh/web/add_forge_now/__init__.py
 create mode 100644 swh/web/add_forge_now/apps.py
 create mode 100644 swh/web/add_forge_now/migrations/0001_initial.py
 create mode 100644 swh/web/add_forge_now/migrations/__init__.py
 create mode 100644 swh/web/add_forge_now/models.py
 create mode 100644 swh/web/add_forge_now/tests/test_migration.py
 create mode 100644 swh/web/api/views/add_forge_now.py
 create mode 100644 swh/web/tests/api/views/test_add_forge_now.py
Changes applied before test
commit d83389b93b7893e7b97fd9d3d0f9e3bff5935265
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Tue Mar 8 15:23:50 2022 +0100

    api: Add endpoint to create an add-forge request
    
    Related to T3990

commit 03101208803501e5178f35d18d171e740ee4ca76
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Tue Mar 8 11:34:44 2022 +0100

    add_forge_now: Bootstrap app and model

See https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1427/ for more details.

Add duplicate create check, return 409 (conflict)

Build is green

Patch application report for D7311 (id=26483)

Could not rebase; Attempt merge onto 7e5b06ff77...

Updating 7e5b06ff..130e9faa
Fast-forward
 swh/web/add_forge_now/__init__.py                |   0
 swh/web/add_forge_now/apps.py                    |  10 ++
 swh/web/add_forge_now/migrations/0001_initial.py | 109 +++++++++++++++++++++
 swh/web/add_forge_now/migrations/__init__.py     |   0
 swh/web/add_forge_now/models.py                  |  71 ++++++++++++++
 swh/web/add_forge_now/tests/test_migration.py    |  62 ++++++++++++
 swh/web/api/urls.py                              |   1 +
 swh/web/api/views/add_forge_now.py               | 108 +++++++++++++++++++++
 swh/web/settings/common.py                       |   1 +
 swh/web/tests/api/views/test_add_forge_now.py    | 117 +++++++++++++++++++++++
 swh/web/tests/utils.py                           |   4 +-
 11 files changed, 482 insertions(+), 1 deletion(-)
 create mode 100644 swh/web/add_forge_now/__init__.py
 create mode 100644 swh/web/add_forge_now/apps.py
 create mode 100644 swh/web/add_forge_now/migrations/0001_initial.py
 create mode 100644 swh/web/add_forge_now/migrations/__init__.py
 create mode 100644 swh/web/add_forge_now/models.py
 create mode 100644 swh/web/add_forge_now/tests/test_migration.py
 create mode 100644 swh/web/api/views/add_forge_now.py
 create mode 100644 swh/web/tests/api/views/test_add_forge_now.py
Changes applied before test
commit 130e9faa9bcab18bc3c76dc898546edb89f3f9b8
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Tue Mar 8 15:23:50 2022 +0100

    api: Add endpoint to create an add-forge request
    
    Related to T3990

commit 03101208803501e5178f35d18d171e740ee4ca76
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Tue Mar 8 11:34:44 2022 +0100

    add_forge_now: Bootstrap app and model

See https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1428/ for more details.

swh/web/api/views/add_forge_now.py
91

that makes the forge_url be the pk of that request object, doesn't it?

Shouldn't it include the forge_type as well?

Shan't we make it explicit in the db model?

swh/web/api/views/add_forge_now.py
91

We considered testing on the forge URL should be sufficient to detect duplicates.

Not sure if we need to make forge_url the primary key.

Anyway, we could still update the model later if it is really needed.

jayeshv added inline comments.
swh/web/api/views/add_forge_now.py
46

Just a thought, not a change request.

Instead of creating functional APIs, can't we go a bit more REST like? I mean to create a resource (ForgeRequest in this case) and add a set of methods around that
My suggestion is
/api/v1/fore-requests/ - GET will get the list of current requests
/api/v1/fore-requests/<id>/ - GET will get a unique forge request
/api/v1/forge-requests/ - POST will create a new request
/api/v1/forge-requests/<id>/ - PUT will update an existing one

swh/web/api/views/add_forge_now.py
46

Currently WIP (T4026, T4027), the following endpoints will be added (not really RESTful but it makes the implementation clearer):

  • POST /api/v1/add-forge/<id>/update: to update a request
  • GET /api/v1/add-forge/list: to list requests
  • GET /api/v1/add-forge/<id>/get: to get a specific request
swh/web/api/views/add_forge_now.py
46

Ok. I have one more question.
In /list and /<id>/get I believe you are going to return the request object. How are you going to handle the personal info fields (email and submitter name) there?. We will expect these details when the UI is accessed by a moderator or the request submitter.
and
Will you be returning the requesthistory in this get API itself, or will there be another endpoint to get that?

76

This POST should also create a RequestHisory object.

swh/web/api/views/add_forge_now.py
76

Maybe this is not needed, for requests in pending RequestHistory can be empty