Paths

Table of Contentst

Differential D2072

deposit.private.api: Expose new endpoints with no collection name
ClosedPublic
Actions

Authored by ardumont on Oct 4 2019, 4:20 PM.

Tags

None

Subscribers

None

Details

Reviewers

Group Reviewers

Commits

rDDEP12e3966b168e: deposit.signals: Scheduler load-deposit task with new endpoints
rDDEP86bc2536b78a: tests: Explicit private tests in their names
rDDEPb762b985077e: deposit.private.api: Expose new endpoints with no collection name

Summary

As those collection name are not functionaly required.

This is a requisite step for the new deposit loader [1].
We decided to implement the new deposit loader with only what's needed (to start clean).

The other functionaly equivalent existing "private" endpoints (which expose the collection name for no good reason) will be dropped soon.

Note:

tests: Explicit private tests in their names

deposit.signals: Scheduler 'load-deposit' task with those new endpoints (<- this is what needs the new endpoints [1]).

[1] https://forge.softwareheritage.org/source/swh-loader-core/browse/package-loader/swh/loader/package/deposit.py$24

Related T2024

Test Plan

tox

Diff Detail

Repository

rDDEP Push deposit

Lint

Automatic diff as part of commit; lint not applicable.

Unit

Automatic diff as part of commit; unit tests not applicable.

Event Timeline

ardumont created this revision.Oct 4 2019, 4:20 PM

Build is green
See https://jenkins.softwareheritage.org/job/DDEP/job/tox/185/ for more details.

Harbormaster completed remote builds in B8109: Diff 6958.Oct 4 2019, 4:22 PM

vlorentz requested changes to this revision.Oct 4 2019, 4:36 PM

vlorentz added inline comments.

swh/deposit/api/private/__init__.py
66–85	Where are the new tests for this method? If there is no need for new tests, then the code of the method is probably redundant with another method. (at the very least, the error message is duplicated)
swh/deposit/api/private/urls.py
38–58	Could be deduplicated by using `r'^((?P<collection_name>[^/]+)/)?(?P<deposit_id>[^/]+)/check/$'` in the regexps above.
swh/deposit/signals.py
95 ↗	(On Diff #6958)	why not toplevel?
swh/deposit/tests/api/test_deposit_read_archive.py
98–125	what happened to these tests?
swh/deposit/tests/api/test_deposit_read_metadata.py
648–661	same

This revision now requires changes to proceed.Oct 4 2019, 4:36 PM

ardumont added inline comments.Oct 4 2019, 5:57 PM

swh/deposit/api/private/__init__.py
66–85	It's here, see the "here we are" below.
swh/deposit/api/private/urls.py
38–58	Yes, but we will remove the new endpoint soon, as in almost immediately. The old endpoints will be replaced by those.
swh/deposit/signals.py
95 ↗	(On Diff #6958)	because i most probably forgot :)
swh/deposit/tests/api/test_deposit_check.py
172	"here we are" (testing the new endpoints)
swh/deposit/tests/api/test_deposit_read_archive.py
97	"here we are"
swh/deposit/tests/api/test_deposit_read_metadata.py
643	"here we are"
swh/deposit/tests/api/test_deposit_update_status.py
133	"here we are"

ardumont added inline comments.Oct 4 2019, 5:59 PM

swh/deposit/api/private/__init__.py
91	The override i mention below about the collection being unused. We allow it in the private api context to not provide the collection.
swh/deposit/tests/api/test_deposit_read_archive.py
98–125	In the context of the "private" api, the collection is not used. It's technically required (because inheritance stuff that we redefined in the SWHPrivateAPI mixin) but it's not used. So we removed those.
swh/deposit/tests/api/test_deposit_read_metadata.py
648–661	same answer, collection is not required, dropping it.

Move import to top-level

Build is green
See https://jenkins.softwareheritage.org/job/DDEP/job/tox/186/ for more details.

Harbormaster completed remote builds in B8115: Diff 6964.Oct 4 2019, 6:04 PM

ardumont edited the summary of this revision. (Show Details)Oct 4 2019, 6:49 PM

ardumont edited the summary of this revision. (Show Details)Oct 5 2019, 2:32 PM

ardumont edited the summary of this revision. (Show Details)Oct 5 2019, 2:34 PM

vlorentz accepted this revision.Oct 7 2019, 11:03 AM

This revision is now accepted and ready to land.Oct 7 2019, 11:03 AM

Closed by commit rDDEPb762b985077e: deposit.private.api: Expose new endpoints with no collection name (authored by ardumont). · Explain WhyOct 7 2019, 2:28 PM

This revision was automatically updated to reflect the committed changes.

ardumont added a commit: rDDEPb762b985077e: deposit.private.api: Expose new endpoints with no collection name.

ardumont added a commit: rDDEP86bc2536b78a: tests: Explicit private tests in their names.

ardumont added a commit: rDDEP12e3966b168e: deposit.signals: Scheduler load-deposit task with new endpoints.

olasd added a reverting change: D2202: Revert "deposit.signals: Scheduler load-deposit task with new endpoints".Oct 30 2019, 3:10 PM

olasd added a reverting change: rDDEPeb60bf5d7f12: Revert "deposit.signals: Scheduler load-deposit task with new endpoints".Oct 30 2019, 4:50 PM

Revision Contents
Changeset List

Path

Size

swh/

deposit/

api/

11 lines

private/

43 lines

deposit_check.py

6 lines

deposit_list.py

4 lines

deposit_read.py

8 lines

deposit_update_status.py

14 lines

21 lines

tests/

api/

test_deposit_check.py

30 lines

test_deposit_read_archive.py

47 lines

test_deposit_read_metadata.py

45 lines

test_deposit_update_status.py

24 lines

Diff 6980

swh/deposit/api/common.py

Loading...

swh/deposit/api/private/init.py

Loading...

swh/deposit/api/private/deposit_check.py

Loading...

swh/deposit/api/private/deposit_list.py

Loading...

swh/deposit/api/private/deposit_read.py

Loading...

swh/deposit/api/private/deposit_update_status.py

Loading...

swh/deposit/api/private/urls.py

Loading...

swh/deposit/tests/api/test_deposit_check.py

Loading...

swh/deposit/tests/api/test_deposit_read_archive.py

Loading...

swh/deposit/tests/api/test_deposit_read_metadata.py

Loading...

swh/deposit/tests/api/test_deposit_update_status.py

Loading...