Page MenuHomeSoftware Heritage
Differential D7327

api: Add endpoint to update an add-forge request
ClosedPublic
Actions

Authored by anlambert on Mar 9 2022, 6:07 PM.

Details

Reviewers
ardumont
Group Reviewers
Reviewers
Maniphest Tasks
T4026: Implement "Update request" API feature
Commits
rDWAPPS1bf17d6a75fd: api: Add endpoint to update an add-forge request
Summary

Related to T4026

Diff Detail

Repository
rDWAPPS Web applications
Branch
add-forge-request-update
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 27408
Build 42886: Phabricator diff pipeline on jenkinsJenkins console · Jenkins
Build 42885: arc lint + arc unit

Event Timeline

anlambert created this revision.Mar 9 2022, 6:07 PM
Herald added a reviewer: Reviewers. · View Herald TranscriptMar 9 2022, 6:07 PM
anlambert added a child revision: D7328: api: Add endpoint to list add-forge requests.Mar 9 2022, 6:07 PM
swh-public-ci added a comment.Mar 9 2022, 6:24 PM

Build is green

Patch application report for D7327 (id=26500)

Could not rebase; Attempt merge onto 51aa6fbc8e...

Merge made by the 'recursive' strategy.
 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                  | 106 +++++++++++
 swh/web/add_forge_now/tests/test_migration.py    |  62 +++++++
 swh/web/api/urls.py                              |   1 +
 swh/web/api/views/add_forge_now.py               | 211 ++++++++++++++++++++++
 swh/web/settings/common.py                       |   1 +
 swh/web/tests/add_forge_now/test_models.py       |  35 ++++
 swh/web/tests/api/views/test_add_forge_now.py    | 220 +++++++++++++++++++++++
 swh/web/tests/utils.py                           |  10 +-
 12 files changed, 762 insertions(+), 3 deletions(-)
 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/add_forge_now/test_models.py
 create mode 100644 swh/web/tests/api/views/test_add_forge_now.py
Changes applied before test
commit 16ed7a54884dc7eefe521df9638a79062e51cf9d
Merge: 51aa6fbc 5233e3c3
Author: Jenkins user <jenkins@localhost>
Date:   Wed Mar 9 17:07:57 2022 +0000

    Merge branch 'diff-target' into HEAD

commit 5233e3c34c67445b04182a93dd9f334233922601
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Wed Mar 9 16:28:03 2022 +0100

    api: Add endpoint to update an add-forge request
    
    Related to T4026

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/1435/ for more details.

Harbormaster completed remote builds in B27385: Diff 26500.Mar 9 2022, 6:24 PM
anlambert requested review of this revision.Mar 9 2022, 6:24 PM
olasd added a subscriber: olasd.Mar 10 2022, 10:38 AM
olasd added inline comments.
swh/web/add_forge_now/models.py
35–69

This could probably be made into a dict?

ardumont added a subscriber: ardumont.Mar 10 2022, 11:13 AM
ardumont added inline comments.
swh/web/add_forge_now/models.py
35–69

yes please ;)

ardumont added inline comments.Mar 10 2022, 11:46 AM
swh/web/api/views/add_forge_now.py
141

do we need the /update/ part?

209

we've got an enum for that in the model object.

vlorentz added a subscriber: vlorentz.Mar 10 2022, 11:49 AM
vlorentz added inline comments.
swh/web/api/views/add_forge_now.py
141

we don't need it, but @anlambert prefers not to have two methods on the same path

anlambert updated this revision to Diff 26507.Mar 10 2022, 11:52 AM

Update: Rework next request statuses allowed code

swh-public-ci added a comment.Mar 10 2022, 12:10 PM

Build has FAILED

Patch application report for D7327 (id=26507)

Could not rebase; Attempt merge onto 51aa6fbc8e...

Merge made by the 'recursive' strategy.
 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                  |  98 +++++++++
 swh/web/add_forge_now/tests/test_migration.py    |  62 ++++++
 swh/web/api/urls.py                              |   1 +
 swh/web/api/views/add_forge_now.py               | 221 +++++++++++++++++++
 swh/web/settings/common.py                       |   1 +
 swh/web/tests/add_forge_now/test_models.py       |  26 +++
 swh/web/tests/api/views/test_add_forge_now.py    | 263 +++++++++++++++++++++++
 swh/web/tests/utils.py                           |  10 +-
 12 files changed, 798 insertions(+), 3 deletions(-)
 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/add_forge_now/test_models.py
 create mode 100644 swh/web/tests/api/views/test_add_forge_now.py
Changes applied before test
commit 25c87d13e70cdc9b42ce94cf2e1adaacf46432f1
Merge: 51aa6fbc b10097c0
Author: Jenkins user <jenkins@localhost>
Date:   Thu Mar 10 10:52:39 2022 +0000

    Merge branch 'diff-target' into HEAD

commit b10097c0457e957bb64ef70e2ea4640112db98cb
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Wed Mar 9 16:28:03 2022 +0100

    api: Add endpoint to update an add-forge request
    
    Related to T4026

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

Link to build: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1437/
See console output for more information: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1437/console

Harbormaster failed remote builds in B27392: Diff 26507!Mar 10 2022, 12:10 PM
anlambert updated this revision to Diff 26509.Mar 10 2022, 1:23 PM

Fix status update

swh-public-ci added a comment.Mar 10 2022, 1:40 PM

Build is green

Patch application report for D7327 (id=26509)

Could not rebase; Attempt merge onto 51aa6fbc8e...

Merge made by the 'recursive' strategy.
 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                  |  98 +++++++++
 swh/web/add_forge_now/tests/test_migration.py    |  62 ++++++
 swh/web/api/urls.py                              |   1 +
 swh/web/api/views/add_forge_now.py               | 225 +++++++++++++++++++
 swh/web/settings/common.py                       |   1 +
 swh/web/tests/add_forge_now/test_models.py       |  26 +++
 swh/web/tests/api/views/test_add_forge_now.py    | 263 +++++++++++++++++++++++
 swh/web/tests/utils.py                           |  10 +-
 12 files changed, 802 insertions(+), 3 deletions(-)
 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/add_forge_now/test_models.py
 create mode 100644 swh/web/tests/api/views/test_add_forge_now.py
Changes applied before test
commit ec3518466f13728d2b82424fb29475f3ba0e325f
Merge: 51aa6fbc 541ac4f8
Author: Jenkins user <jenkins@localhost>
Date:   Thu Mar 10 12:23:44 2022 +0000

    Merge branch 'diff-target' into HEAD

commit 541ac4f8d0dc0bba4ae7c8f268c488776307f17f
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Wed Mar 9 16:28:03 2022 +0100

    api: Add endpoint to update an add-forge request
    
    Related to T4026

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/1439/ for more details.

Harbormaster completed remote builds in B27394: Diff 26509.Mar 10 2022, 1:40 PM
ardumont accepted this revision as: ardumont.Mar 10 2022, 6:05 PM
ardumont added 1 blocking reviewer(s): Reviewers.

great, some small comment inline.

swh/web/add_forge_now/models.py
57

i scratched my head on this at first (and then it's an enum so i guess it makes sense ;)

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

a small comment to explain that as the request got updated to waiting-for-feedback (by the other faster thread), it no longer can be updated to rejected. It's not immediately apparent here.

vsellier added a subscriber: vsellier.Mar 10 2022, 7:55 PM
vsellier added inline comments.
swh/web/add_forge_now/models.py
38

I just realized that if the forge admin never replies to the message, we will not be able to close the request.
I don't know if it makes sense.

vsellier added inline comments.Mar 10 2022, 8:21 PM
swh/web/add_forge_now/models.py
38
  • not able :)
ardumont added inline comments.Mar 11 2022, 8:37 AM
swh/web/add_forge_now/models.py
38

good catch.
Might be it's ok to stay indefinitely in that waiting for feedback state (depending on the occurrence frequency of those requests...).
Might be it'd be best if we could have some kind of garbage routine which suspends or rejects such staled requests after a definite amount of time (and that would be explained in the docs as well to avoid surprised users) or something.

That should probably come up in the afternoon discussion.

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

ok-ish i guess (that's not really an argument though).

Is that because of a technical issue inside the decorator stuff which we don't want to dig into or something? (just asking to be aware of such "details")

212

IIRC, the enum stuff (previous comment ^) is taken care of in another diff.

vlorentz added inline comments.Mar 11 2022, 9:51 AM
swh/web/add_forge_now/models.py
57

This is because mypy does not understand enum.Enum has a metaclass that transforms class attributes to variants; so it infers the RHS of the next_statuses assignment as Dict[str, str]

anlambert updated this revision to Diff 26524.Mar 11 2022, 10:09 AM

Update: Address @ardumont comments

swh-public-ci added a comment.Mar 11 2022, 10:11 AM

Build has FAILED

Patch application report for D7327 (id=26524)

Could not rebase; Attempt merge onto cdcc1efdfb...

Merge made by the 'recursive' strategy.
 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                  |  98 +++++++++
 swh/web/add_forge_now/tests/test_migration.py    |  62 ++++++
 swh/web/api/urls.py                              |   1 +
 swh/web/api/views/add_forge_now.py               | 226 +++++++++++++++++++
 swh/web/settings/common.py                       |   1 +
 swh/web/tests/add_forge_now/test_models.py       |  26 +++
 swh/web/tests/api/views/test_add_forge_now.py    | 267 +++++++++++++++++++++++
 swh/web/tests/utils.py                           |  10 +-
 12 files changed, 807 insertions(+), 3 deletions(-)
 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/add_forge_now/test_models.py
 create mode 100644 swh/web/tests/api/views/test_add_forge_now.py
Changes applied before test
commit 5cc2c8ff2b35f317cdaf9eace58e6040d43f0e61
Merge: cdcc1efd 1bf17d6a
Author: Jenkins user <jenkins@localhost>
Date:   Fri Mar 11 09:10:37 2022 +0000

    Merge branch 'diff-target' into HEAD

commit 1bf17d6a75fd7265b1da9c9e87220efa62869c93
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Wed Mar 9 16:28:03 2022 +0100

    api: Add endpoint to update an add-forge request
    
    Related to T4026

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

Link to build: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1444/
See console output for more information: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1444/console

Harbormaster failed remote builds in B27408: Diff 26524!Mar 11 2022, 10:11 AM
swh-public-ci added a comment.Mar 11 2022, 10:15 AM

Build has FAILED

Link to build: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1447/
See console output for more information: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1447/console

swh-public-ci added a comment.Mar 11 2022, 10:31 AM

Build is green

Patch application report for D7327 (id=26524)

Could not rebase; Attempt merge onto cdcc1efdfb...

Merge made by the 'recursive' strategy.
 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                  |  98 +++++++++
 swh/web/add_forge_now/tests/test_migration.py    |  62 ++++++
 swh/web/api/urls.py                              |   1 +
 swh/web/api/views/add_forge_now.py               | 226 +++++++++++++++++++
 swh/web/settings/common.py                       |   1 +
 swh/web/tests/add_forge_now/test_models.py       |  26 +++
 swh/web/tests/api/views/test_add_forge_now.py    | 267 +++++++++++++++++++++++
 swh/web/tests/utils.py                           |  10 +-
 12 files changed, 807 insertions(+), 3 deletions(-)
 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/add_forge_now/test_models.py
 create mode 100644 swh/web/tests/api/views/test_add_forge_now.py
Changes applied before test
commit 3d4b28e64db0b8df8da10aa4c3f93fe8c487ba18
Merge: cdcc1efd 1bf17d6a
Author: Jenkins user <jenkins@localhost>
Date:   Fri Mar 11 09:15:30 2022 +0000

    Merge branch 'diff-target' into HEAD

commit 1bf17d6a75fd7265b1da9c9e87220efa62869c93
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Wed Mar 9 16:28:03 2022 +0100

    api: Add endpoint to update an add-forge request
    
    Related to T4026

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/1448/ for more details.

Harbormaster completed remote builds in B27408: Diff 26524.Mar 11 2022, 10:31 AM
ardumont accepted this revision.Mar 11 2022, 3:13 PM
This revision is now accepted and ready to land.Mar 11 2022, 3:13 PM
anlambert closed this revision.Mar 11 2022, 4:03 PM

Landed in 1bf17d6a75fd.

anlambert added a commit: rDWAPPS1bf17d6a75fd: api: Add endpoint to update an add-forge request.Mar 16 2022, 2:18 PM

Revision Contents
Changeset List

PathSizeCoverage (All)Coverage (Touched)
swh/
web/
add_forge_now/
27 lines100%
Loading...
api/
views/
120 lines97%
Loading...
tests/
add_forge_now/
26 lines100%
Loading...
api/
views/
156 lines100%
Loading...
6 lines100%
Loading...

Diff 26524

View Options

swh/web/add_forge_now/models.py

Loading...
View Options

swh/web/api/views/add_forge_now.py

Loading...
View Options

swh/web/tests/add_forge_now/test_models.py

Loading...
View Options

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

Loading...
View Options

swh/web/tests/utils.py

Loading...
About · Developer information · Legal