Page MenuHomeSoftware Heritage

Make POST SE-IRI schedule a check task.
ClosedPublic

Authored by vlorentz on Nov 25 2020, 1:10 PM.

Details

Summary

Otherwise, it just marks the deposit as 'deposited' but the deposit
will never be checked, so it will never be loaded.

Additionally, this commit deduplicates code between 'POST SE-IRI'
and the other POST/PUT endpoints.

Depends on D4589

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

swh/deposit/tests/api/test_deposit_schedule.py
64–75

this was moved to assert_task_for_deposit

Build is green

Patch application report for D4590 (id=16293)

Could not rebase; Attempt merge onto f612cdc00e...

Updating f612cdc0..f0762e99
Fast-forward
 swh/deposit/api/collection.py                    |  54 +++++-
 swh/deposit/api/common.py                        | 209 +++++++++++------------
 swh/deposit/api/content.py                       |   4 +-
 swh/deposit/api/edit.py                          |  13 +-
 swh/deposit/api/edit_media.py                    |  15 +-
 swh/deposit/api/private/__init__.py              |  17 +-
 swh/deposit/api/private/deposit_check.py         |   7 +-
 swh/deposit/api/private/deposit_read.py          |  11 +-
 swh/deposit/api/private/deposit_update_status.py |  10 +-
 swh/deposit/api/state.py                         |   4 +-
 swh/deposit/api/sword_edit.py                    |  13 +-
 swh/deposit/tests/api/test_deposit_schedule.py   |  84 +++++++--
 swh/deposit/tests/api/test_deposit_update.py     |   4 +-
 swh/deposit/tests/conftest.py                    |  22 +--
 14 files changed, 276 insertions(+), 191 deletions(-)
Changes applied before test
commit f0762e99f8a8983c7f128235b9776830a1e40f63
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Wed Nov 25 13:08:08 2020 +0100

    Make POST SE-IRI schedule a check task.
    
    Otherwise, it just marks the deposit as 'deposited' but the deposit
    will never be checked, so it will never be loaded.
    
    Additionally, this commit deduplicates code between 'POST SE-IRI'
    and the other POST/PUT endpoints.

commit ac2a5f4c958d70ccf4b669fd69a441d7fec55577
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Wed Nov 25 12:59:54 2020 +0100

    conftest: Set HTTP_IN_PROGRESS to True when creating partial deposits.
    
    conftest is cheating by hard-resetting the deposit status to partial
    after creating a complete deposit; but it doesn't make sense for
    a real scenario.
    
    Additionally, a future commit will add a test relying on the
    'partial_deposit' not scheduling a task, so the HTTP_IN_PROGRESS
    header will need to be True at that point.

commit fb1c19d942ed83a2335e4a5888d6475f2d38df5b
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Wed Nov 25 11:53:27 2020 +0100

    Make APIBase.get_client return a non-Optional.
    
    As it already could never return None.

commit 42ad8df54b4e1b28dab51e934393db3a9af74862
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Wed Nov 25 11:49:59 2020 +0100

    Don't set APIBase._client in check()
    
    Instead, make it really an internal attribute with a method
    that uses as a cache.
    
    Motivation: a method named 'checks' shouldn't set attributes.

commit df87127d16c478767cd418a9d696017fdae5aed0
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Wed Nov 25 11:30:55 2020 +0100

    Remove attribute APIBase._collection
    
    Motivation:
    
    1. It was used only internally in checks() and in 'POST Col-IRI'
    2. A method named "check" should not set attributes

commit bd46805412a8d41b573aee776ac0a6b40aafa362
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Wed Nov 25 11:17:54 2020 +0100

    Check collection name when calling get_deposit_by_id, if relevant.

commit 85486a79e014f681aaab844998b1fa98ccc92b49
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Tue Nov 24 16:32:42 2020 +0100

    Move Deposit creation from api/common.py to api/collection.py
    
    Motivation:
    
    1. it's not an operation in common, only 'POST Col-IRI' may create a deposit
    2. it makes the BaseAPI._deposit_put function simpler and less complex
    
    This looks like a very long commit, but there are actually four small
    changes. In order:
    
    1. actually move that creation (the large block of code was moved
       from one file to the other)
    
    2. make the 'deposit_id' argument of most functions of 'int' type
       instead of 'Optional[int]', since we now get the id very early
       by creating it in the Col-IRI view.
    
    3. unfortunately, this means creating a Deposit and committing it
       directly to the DB, which changes the behavior of the API, as it used
       to run checks *before* committing to the DB.
       Therefore, instead of immediately committing to get the id, it passes
       the Deposit object, which will be committed later by _deposit_put if
       all checks pass.
       This means changing the 'deposit_id: int' argument of many internal
       functions to 'deposit: Deposit'. This is why the diff looks big,
       but it's actually a trivial change
    
    4. updated test_add_metadata_to_unknown_deposit because the API now returns
       the right error message, as a side-effect.

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

Build is green

Patch application report for D4590 (id=16295)

Rebasing onto f612cdc00e...

Current branch diff-target is up to date.
Changes applied before test
commit 96cd390557c38dd14c2baec72940489cb1e512aa
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Wed Nov 25 13:08:08 2020 +0100

    Make POST SE-IRI schedule a check task.
    
    Otherwise, it just marks the deposit as 'deposited' but the deposit
    will never be checked, so it will never be loaded.
    
    Additionally, this commit deduplicates code between 'POST SE-IRI'
    and the other POST/PUT endpoints.

commit ac2a5f4c958d70ccf4b669fd69a441d7fec55577
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Wed Nov 25 12:59:54 2020 +0100

    conftest: Set HTTP_IN_PROGRESS to True when creating partial deposits.
    
    conftest is cheating by hard-resetting the deposit status to partial
    after creating a complete deposit; but it doesn't make sense for
    a real scenario.
    
    Additionally, a future commit will add a test relying on the
    'partial_deposit' not scheduling a task, so the HTTP_IN_PROGRESS
    header will need to be True at that point.

commit fb1c19d942ed83a2335e4a5888d6475f2d38df5b
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Wed Nov 25 11:53:27 2020 +0100

    Make APIBase.get_client return a non-Optional.
    
    As it already could never return None.

commit 42ad8df54b4e1b28dab51e934393db3a9af74862
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Wed Nov 25 11:49:59 2020 +0100

    Don't set APIBase._client in check()
    
    Instead, make it really an internal attribute with a method
    that uses as a cache.
    
    Motivation: a method named 'checks' shouldn't set attributes.

commit df87127d16c478767cd418a9d696017fdae5aed0
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Wed Nov 25 11:30:55 2020 +0100

    Remove attribute APIBase._collection
    
    Motivation:
    
    1. It was used only internally in checks() and in 'POST Col-IRI'
    2. A method named "check" should not set attributes

commit bd46805412a8d41b573aee776ac0a6b40aafa362
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Wed Nov 25 11:17:54 2020 +0100

    Check collection name when calling get_deposit_by_id, if relevant.

commit 85486a79e014f681aaab844998b1fa98ccc92b49
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Tue Nov 24 16:32:42 2020 +0100

    Move Deposit creation from api/common.py to api/collection.py
    
    Motivation:
    
    1. it's not an operation in common, only 'POST Col-IRI' may create a deposit
    2. it makes the BaseAPI._deposit_put function simpler and less complex
    
    This looks like a very long commit, but there are actually four small
    changes. In order:
    
    1. actually move that creation (the large block of code was moved
       from one file to the other)
    
    2. make the 'deposit_id' argument of most functions of 'int' type
       instead of 'Optional[int]', since we now get the id very early
       by creating it in the Col-IRI view.
    
    3. unfortunately, this means creating a Deposit and committing it
       directly to the DB, which changes the behavior of the API, as it used
       to run checks *before* committing to the DB.
       Therefore, instead of immediately committing to get the id, it passes
       the Deposit object, which will be committed later by _deposit_put if
       all checks pass.
       This means changing the 'deposit_id: int' argument of many internal
       functions to 'deposit: Deposit'. This is why the diff looks big,
       but it's actually a trivial change
    
    4. updated test_add_metadata_to_unknown_deposit because the API now returns
       the right error message, as a side-effect.

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

Build is green

Patch application report for D4590 (id=16296)

Could not rebase; Attempt merge onto f612cdc00e...

Updating f612cdc0..96cd3905
Fast-forward
 swh/deposit/api/collection.py                    |  54 +++++-
 swh/deposit/api/common.py                        | 211 +++++++++++------------
 swh/deposit/api/content.py                       |   4 +-
 swh/deposit/api/edit.py                          |  13 +-
 swh/deposit/api/edit_media.py                    |  15 +-
 swh/deposit/api/private/__init__.py              |  17 +-
 swh/deposit/api/private/deposit_check.py         |   7 +-
 swh/deposit/api/private/deposit_read.py          |  11 +-
 swh/deposit/api/private/deposit_update_status.py |  10 +-
 swh/deposit/api/state.py                         |   4 +-
 swh/deposit/api/sword_edit.py                    |  13 +-
 swh/deposit/tests/api/test_deposit_schedule.py   |  84 +++++++--
 swh/deposit/tests/api/test_deposit_update.py     |   4 +-
 swh/deposit/tests/conftest.py                    |  22 +--
 14 files changed, 277 insertions(+), 192 deletions(-)
Changes applied before test
commit 96cd390557c38dd14c2baec72940489cb1e512aa
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Wed Nov 25 13:08:08 2020 +0100

    Make POST SE-IRI schedule a check task.
    
    Otherwise, it just marks the deposit as 'deposited' but the deposit
    will never be checked, so it will never be loaded.
    
    Additionally, this commit deduplicates code between 'POST SE-IRI'
    and the other POST/PUT endpoints.

commit ac2a5f4c958d70ccf4b669fd69a441d7fec55577
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Wed Nov 25 12:59:54 2020 +0100

    conftest: Set HTTP_IN_PROGRESS to True when creating partial deposits.
    
    conftest is cheating by hard-resetting the deposit status to partial
    after creating a complete deposit; but it doesn't make sense for
    a real scenario.
    
    Additionally, a future commit will add a test relying on the
    'partial_deposit' not scheduling a task, so the HTTP_IN_PROGRESS
    header will need to be True at that point.

commit fb1c19d942ed83a2335e4a5888d6475f2d38df5b
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Wed Nov 25 11:53:27 2020 +0100

    Make APIBase.get_client return a non-Optional.
    
    As it already could never return None.

commit 42ad8df54b4e1b28dab51e934393db3a9af74862
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Wed Nov 25 11:49:59 2020 +0100

    Don't set APIBase._client in check()
    
    Instead, make it really an internal attribute with a method
    that uses as a cache.
    
    Motivation: a method named 'checks' shouldn't set attributes.

commit df87127d16c478767cd418a9d696017fdae5aed0
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Wed Nov 25 11:30:55 2020 +0100

    Remove attribute APIBase._collection
    
    Motivation:
    
    1. It was used only internally in checks() and in 'POST Col-IRI'
    2. A method named "check" should not set attributes

commit bd46805412a8d41b573aee776ac0a6b40aafa362
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Wed Nov 25 11:17:54 2020 +0100

    Check collection name when calling get_deposit_by_id, if relevant.

commit 85486a79e014f681aaab844998b1fa98ccc92b49
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Tue Nov 24 16:32:42 2020 +0100

    Move Deposit creation from api/common.py to api/collection.py
    
    Motivation:
    
    1. it's not an operation in common, only 'POST Col-IRI' may create a deposit
    2. it makes the BaseAPI._deposit_put function simpler and less complex
    
    This looks like a very long commit, but there are actually four small
    changes. In order:
    
    1. actually move that creation (the large block of code was moved
       from one file to the other)
    
    2. make the 'deposit_id' argument of most functions of 'int' type
       instead of 'Optional[int]', since we now get the id very early
       by creating it in the Col-IRI view.
    
    3. unfortunately, this means creating a Deposit and committing it
       directly to the DB, which changes the behavior of the API, as it used
       to run checks *before* committing to the DB.
       Therefore, instead of immediately committing to get the id, it passes
       the Deposit object, which will be committed later by _deposit_put if
       all checks pass.
       This means changing the 'deposit_id: int' argument of many internal
       functions to 'deposit: Deposit'. This is why the diff looks big,
       but it's actually a trivial change
    
    4. updated test_add_metadata_to_unknown_deposit because the API now returns
       the right error message, as a side-effect.

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

ardumont added a subscriber: ardumont.

Nice catch.

That's actually a bug fix.

This revision is now accepted and ready to land.Nov 25 2020, 1:57 PM
This revision was automatically updated to reflect the committed changes.