Refactor common reading behavior between deposit read and check
ClosedPublic
Actions

Authored by ardumont on Jul 13 2018, 5:41 PM.

Details

Reviewers

Commits

rDDEP9dc5dd827751: api.private: Refactor common behavior for reading deposit requests
rDDEP9b4eb1e1b433: api.private: Refactor common behavior for reading metadata
rDDEP8b1d19030e7b: deposit.tests: Do not override external identifier in metadata

Summary

deposit.tests: Do not override external identifier in metadata
api.private: Refactor common behavior for reading deposit requests
api.private: Refactor common behavior for reading metadata

Test Plan

Tests ok

Diff Detail

Repository

rDDEP Push deposit

Branch

master

Lint

No Linters Available

Unit

No Unit Test Coverage

Build Status

Buildable 1341
Build 1685: arc lint + arc unit

Event Timeline

ardumont created this revision.Jul 13 2018, 5:41 PM

Harbormaster completed remote builds in B1322: Diff 1182.Jul 13 2018, 5:41 PM

moranegg accepted this revision.Jul 16 2018, 11:11 AM

moranegg added inline comments.

swh/deposit/api/private/deposit_check.py
21	Here there are 2 types of archive error but in the tests there's invalid, unreadable and unsupported.. Maybe you should keep the same logic and naming if archive missing is one of the three...
22	Maybe keeping all metadata errors together, so move to line 19
33	Here is a place where the possible fails of the check can be documented

This revision is now accepted and ready to land.Jul 16 2018, 11:11 AM

Update?

deposit_check: Reject deposit containing a single archive
deposit_check: Add test around deposit with 1 archive rejection
deposit_status: Update the deposit status endpoint
swh.deposit.models: Reuse status variable

In D381#7353, @ardumont wrote:

Update?

deposit_check: Reject deposit containing a single archive

deposit_check: Add test around deposit with 1 archive rejection

deposit_status: Update the deposit status endpoint

swh.deposit.models: Reuse status variable

nope, wrong manip.

Revert to initial diff

deposit.tests: Do not override external identifier in metadata
api.private: Refactor common behavior for reading deposit requests
api.private: Refactor common behavior for reading metadata

Merged.

ardumont added inline comments.Jul 16 2018, 3:35 PM

swh/deposit/api/private/deposit_check.py
21	mmm, i finally understand you. The diff is not showing correctly the content of the diffs. Also, i might be at fault here since i updated that diff with the content of D380 and that's not what i wanted to do... Maybe you should keep the same logic and naming if archive missing is one of the three... I did but we do not see it... (D380) neither... We need to look at the different diff ids in D380 to actually see what we want...