Details

Reviewers

anlambert

Group Reviewers

Reviewers

Maniphest Tasks

T3213: Enable save code now of software source code archives for specific users

Commits

rDWAPPS450988b96900: Save code now: Check origin exist prior to save request

Summary

This slightly modifies the current save code now workflow. Prior to register a new
origin url for "save code now", this first ensures said origin exists (through a HEAD
request done server side).

If the origin does not exist, an explanation notification is provided to the user.
Otherwise, the origin is submitted for registration as usual.

This should decrease the number of loading visits ending up in not_found status. This
also prepares work for the incoming new bundle visit type (which needs both that check
and the HEAD request anyway to fetch some more information).

Related to T3213

Test Plan

tox
make test-frontend

Diff Detail

Repository

rDWAPPS Web applications

Branch

master

Lint

No Linters Available

Unit

No Unit Test Coverage

Build Status

Buildable 21182
Build 32879: Phabricator diff pipeline on jenkins	Jenkins console · Jenkins
Build 32878: arc lint + arc unit

Unit TestsFailed
View All

	Time	Test
	0 ms	Jenkins > should show error when the origin does not exist (status: 400)::Tests / Cypress tests / Run cypress tests / Origin Save Tests should show error when the origin does not exist (status: 400) AssertionError: Timed out retrying after 20000ms: expected '<div.alert.alert-danger.>' to contain 'The provided url does not exist!' at checkAlertVisible (http://localhost:5004/__cypress/tests?p=cypress/integration/origin-save.spec.js:123:115) at Context.eval (http://localhost:5004/__cypress/tests?p=cypress/integration/origin-save.spec.js:349:7)
	293 ms	Jenkins > .tox.py3.lib.python3.7.site-packages.swh.web.tests.admin.test_origin_save::Tests / Python tests / test_accept_pending_save_request
	963 ms	Jenkins > .tox.py3.lib.python3.7.site-packages.swh.web.tests.admin.test_origin_save::Tests / Python tests / test_add_authorized_origin_url
	120 ms	Jenkins > .tox.py3.lib.python3.7.site-packages.swh.web.tests.admin.test_origin_save::Tests / Python tests / test_add_unauthorized_origin_url
	314 ms	Jenkins > .tox.py3.lib.python3.7.site-packages.swh.web.tests.admin.test_origin_save::Tests / Python tests / test_reject_pending_save_request
		View Full Test Results (1 Failed · 649 Passed · 5 Skipped)

Event Timeline

ardumont created this revision.Apr 30 2021, 12:36 PM

ardumont retitled this revision from Open endpoint to check origin submitted for save code now exists to Save code now: Check origin exist prior to save request.Apr 30 2021, 12:40 PM

Build has FAILED

Patch application report for D5653 (id=20187)

Rebasing onto 03cfd89a1d...

First, rewinding head to replay your work on top of it...
Applying: Open endpoint to check origin submitted for save code now exists

Changes applied before test

commit 9721e72f76b96c173ad6348cda2ae9fcb1a81520
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Thu Apr 29 19:03:09 2021 +0200

    Open endpoint to check origin submitted for save code now exists
    
    This slightly modifies the current save code now workflow. Prior to register a new
    origin url for "save code now", this first ensures said origin exists (through a HEAD
    request done server side).
    
    If the origin does not exist, an explanation notification is provided to the user.
    Otherwise, the origin is submitted for registration as usual.
    
    This should decrease the number of visit with not_found status. This also prepares work
    for the incoming new bundle visit type (which needs that check).
    
    Related to T3213

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

Harbormaster returned this revision to the author for changes because remote builds failed.Apr 30 2021, 12:44 PM

Harbormaster failed remote builds in B21177: Diff 20187!

Rework commit message
Fix forgotten updates on tests server side

Build has FAILED

Patch application report for D5653 (id=20193)

Rebasing onto 03cfd89a1d...

First, rewinding head to replay your work on top of it...
Applying: Save code now: Check origin exist prior to save request

Changes applied before test

commit 6a28f79d0f6bb9771237725ba4a3ea4e3ce50bdd
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Thu Apr 29 19:03:09 2021 +0200

    Save code now: Check origin exist prior to save request
    
    This slightly modifies the current save code now workflow. Prior to register a new
    origin url for "save code now", this first ensures said origin exists (through a HEAD
    request done server side).
    
    If the origin does not exist, an explanation notification is provided to the user.
    Otherwise, the origin is submitted for registration as usual.
    
    This should decrease the number of visit with not_found status. This also prepares work
    for the incoming new bundle visit type (which needs that check).
    
    Related to T3213

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

Harbormaster returned this revision to the author for changes because remote builds failed.Apr 30 2021, 2:31 PM

Harbormaster failed remote builds in B21182: Diff 20193!

ardumont added inline comments.Apr 30 2021, 2:43 PM

assets/src/bundles/save/index.js
203	For some reason, my docker experiment showed the actual desired error message (the one raised from the python code) was in the `reason` key [1] and not the `detail` key as the other conditional demonstrates. [1]

Fix js test (it's not as easy as it sounds or so i'm experiencing ¯\_(ツ)_/¯ ;)

ardumont added inline comments.Apr 30 2021, 3:11 PM

assets/src/bundles/save/index.js
203	also, we could do that to avoid modifying the utility function test below (to open the `reason` key)...

Build has FAILED

Patch application report for D5653 (id=20196)

Rebasing onto 03cfd89a1d...

First, rewinding head to replay your work on top of it...
Applying: Save code now: Check origin exist prior to save request

Changes applied before test

commit fe65a748aedab14bff09007d28d70cc0ea519f61
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Thu Apr 29 19:03:09 2021 +0200

    Save code now: Check origin exist prior to save request
    
    This slightly modifies the current save code now workflow. Prior to register a new
    origin url for "save code now", this first ensures said origin exists (through a HEAD
    request done server side).
    
    If the origin does not exist, an explanation notification is provided to the user.
    Otherwise, the origin is submitted for registration as usual.
    
    This should decrease the number of visit with not_found status. This also prepares work
    for the incoming new bundle visit type (which needs that check).
    
    Related to T3213

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

Harbormaster returned this revision to the author for changes because remote builds failed.Apr 30 2021, 3:19 PM

Harbormaster failed remote builds in B21185: Diff 20196!

Build has FAILED

lol, now what?!

Jenkins > should show not found for content::Tests / Cypress tests / Run cypress tests / Test origin-search Test invalid SWHIDs should show not found for content

ok, unrelated.

Build is green

Patch application report for D5653 (id=20196)

Rebasing onto 03cfd89a1d...

First, rewinding head to replay your work on top of it...
Applying: Save code now: Check origin exist prior to save request

Changes applied before test

commit d7aae1aa5ee7618a454232b8bfa395f312a00092
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Thu Apr 29 19:03:09 2021 +0200

    Save code now: Check origin exist prior to save request
    
    This slightly modifies the current save code now workflow. Prior to register a new
    origin url for "save code now", this first ensures said origin exists (through a HEAD
    request done server side).
    
    If the origin does not exist, an explanation notification is provided to the user.
    Otherwise, the origin is submitted for registration as usual.
    
    This should decrease the number of visit with not_found status. This also prepares work
    for the incoming new bundle visit type (which needs that check).
    
    Related to T3213

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

Harbormaster completed remote builds in B21185: Diff 20196.Apr 30 2021, 3:32 PM

ardumont requested review of this revision.Apr 30 2021, 3:32 PM

ardumont edited the summary of this revision. (Show Details)Apr 30 2021, 3:43 PM

anlambert added inline comments.Apr 30 2021, 4:37 PM

assets/src/bundles/save/index.js
203	Hmm, this detail field looks weird to me, I have the feeling that it should be reason everywhere, let me check the git history.

ardumont added inline comments.Apr 30 2021, 4:40 PM

assets/src/bundles/save/index.js
202	also note that i did not want to actually change that ^ (it just stayed and i missed to remove it). Since the build finally passed green, i waited for some comments first to update it...
203	i agree. It feels like i should align everything to that. I did not check though and unshamingly relying on your memory for this ;)

anlambert added inline comments.Apr 30 2021, 4:47 PM

assets/src/bundles/save/index.js
202	Turns out I removed the python code that was returning the dict with the detail field in a really recent commit: https://forge.softwareheritage.org/rDWAPPSf9b0b666d512564fddfc2fda65b2975fa0ea75c3#change-hmkCToN48Oc7 Now that we are using the Web API endpoint to create requests, the error detail field is now `reason` for all error responses.

ardumont added inline comments.Apr 30 2021, 4:54 PM

assets/src/bundles/save/index.js
202	great then, i'll unify along the way (including the tests).

Looks good to me, some test comments need to be fixed though.

swh/web/common/origin_save.py
168	I think you can drop the `is True`.
swh/web/tests/common/test_origin_save.py
336	Comment is not correct here.
352	same here

This revision is now accepted and ready to land.Apr 30 2021, 4:55 PM

Rebase

Adapt according to review:

Adapt the reason error inconsistency
Fix test docstring
Simplify the is True stanza

ardumont added inline comments.Apr 30 2021, 5:04 PM

swh/web/common/origin_save.py
168	indeed, i wanted it to stand out but the simple version sounds enough today ;)

Build is green

Patch application report for D5653 (id=20198)

Rebasing onto 03cfd89a1d...

Current branch diff-target is up to date.

Changes applied before test

commit 450988b969007c9c8264bfe973c806bf5a68425f
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Thu Apr 29 19:03:09 2021 +0200

    Save code now: Check origin exist prior to save request
    
    This slightly modifies the current save code now workflow. Prior to register a new
    origin url for "save code now", this first ensures said origin exists (through a HEAD
    request done server side).
    
    If the origin does not exist, an explanation notification is provided to the user.
    Otherwise, the origin is submitted for registration as usual.
    
    This should decrease the number of loading visits ending up in not_found status. This
    also prepares work for the incoming new bundle visit type (which needs both that check
    and the HEAD request anyway to fetch some more information).
    
    Related to T3213

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

Harbormaster completed remote builds in B21187: Diff 20198.Apr 30 2021, 5:11 PM

Closed by commit rDWAPPS450988b96900: Save code now: Check origin exist prior to save request (authored by ardumont). · Explain WhyApr 30 2021, 5:12 PM

This revision was automatically updated to reflect the committed changes.

ardumont added a commit: rDWAPPS450988b96900: Save code now: Check origin exist prior to save request.

Save code now: Check origin exist prior to save request
ClosedPublic
Actions

Details

Diff Detail

Unit TestsFailed
View All

Event Timeline

Patch application report for D5653 (id=20187)

Changes applied before test

Patch application report for D5653 (id=20193)

Changes applied before test

Patch application report for D5653 (id=20196)

Changes applied before test

Patch application report for D5653 (id=20196)

Changes applied before test

Patch application report for D5653 (id=20198)

Changes applied before test

Revision Contents
Changeset List

Diff 20193

assets/src/bundles/save/index.js

cypress/integration/origin-save.spec.js

swh/web/common/origin_save.py

swh/web/common/typing.py

swh/web/tests/admin/test_origin_save.py

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

swh/web/tests/common/test_origin_save.py

Save code now: Check origin exist prior to save requestClosedPublicActions

Details

Diff Detail

Unit TestsFailedView All

Event Timeline

Patch application report for D5653 (id=20187)

Changes applied before test

Patch application report for D5653 (id=20193)

Changes applied before test

Patch application report for D5653 (id=20196)

Changes applied before test

Patch application report for D5653 (id=20196)

Changes applied before test

Patch application report for D5653 (id=20198)

Changes applied before test

Revision ContentsChangeset List

Diff 20193

assets/src/bundles/save/index.js

cypress/integration/origin-save.spec.js

swh/web/common/origin_save.py

swh/web/common/typing.py

swh/web/tests/admin/test_origin_save.py

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

swh/web/tests/common/test_origin_save.py

Save code now: Check origin exist prior to save request
ClosedPublic
Actions

Unit TestsFailed
View All

Revision Contents
Changeset List