Page MenuHomeSoftware Heritage

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

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

Details

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 swh-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.Fri, Oct 4, 4:20 PM
vlorentz requested changes to this revision.Fri, Oct 4, 4:36 PM
vlorentz added inline comments.
swh/deposit/api/private/__init__.py
67–86

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
39–59

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.Fri, Oct 4, 4:36 PM
ardumont added inline comments.Fri, Oct 4, 5:57 PM
swh/deposit/api/private/__init__.py
67–86

It's here, see the "here we are" below.

swh/deposit/api/private/urls.py
39–59

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
173

"here we are"
(testing the new endpoints)

swh/deposit/tests/api/test_deposit_read_archive.py
98

"here we are"

swh/deposit/tests/api/test_deposit_read_metadata.py
644

"here we are"

swh/deposit/tests/api/test_deposit_update_status.py
134

"here we are"

ardumont added inline comments.Fri, Oct 4, 5:59 PM
swh/deposit/api/private/__init__.py
92

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.

ardumont updated this revision to Diff 6964.Fri, Oct 4, 6:02 PM

Move import to top-level

ardumont edited the summary of this revision. (Show Details)Fri, Oct 4, 6:49 PM
ardumont edited the summary of this revision. (Show Details)Sat, Oct 5, 2:32 PM
ardumont edited the summary of this revision. (Show Details)Sat, Oct 5, 2:34 PM
vlorentz accepted this revision.Mon, Oct 7, 11:03 AM
This revision is now accepted and ready to land.Mon, Oct 7, 11:03 AM