Page MenuHomeSoftware Heritage

Remove return values of process_put and process_delete.
ClosedPublic

Authored by vlorentz on Nov 18 2020, 2:15 PM.

Details

Summary

They were only used for the special "error" key, which no longer
exists, so they are useless now.

Depends on D4505.

Diff Detail

Repository
rDDEP Push deposit
Branch
reorganize-sword
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 17190
Build 26542: Phabricator diff pipeline on jenkinsJenkins console · Jenkins
Build 26541: arc lint + arc unit

Event Timeline

Build has FAILED

Patch application report for D4507 (id=15985)

Could not rebase; Attempt merge onto a67ed6b06b...

Updating a67ed6b0..00b3af02
Fast-forward
 swh/deposit/api/{deposit.py => collection.py}      |  12 +-
 swh/deposit/api/common.py                          | 315 ++++++++++-----------
 swh/deposit/api/{deposit_content.py => content.py} |  16 +-
 swh/deposit/api/deposit_update.py                  | 277 ------------------
 swh/deposit/api/edit.py                            | 139 +++++++++
 swh/deposit/api/edit_media.py                      |  96 +++++++
 swh/deposit/api/private/__init__.py                |   8 +-
 swh/deposit/api/private/deposit_update_status.py   |  26 +-
 swh/deposit/api/service_document.py                |   2 +-
 swh/deposit/api/{deposit_status.py => state.py}    |  10 +-
 swh/deposit/api/sword_edit.py                      |  82 ++++++
 swh/deposit/api/urls.py                            |  44 +--
 swh/deposit/config.py                              |   3 +-
 swh/deposit/errors.py                              |  35 ++-
 swh/deposit/settings/common.py                     |   1 +
 swh/deposit/templates/deposit/deposit_receipt.xml  |   4 +-
 .../api/{test_deposit.py => test_collection.py}    |   4 +-
 ...eposit_atom.py => test_collection_post_atom.py} |  11 +-
 ...it_binary.py => test_collection_post_binary.py} |  17 +-
 ...etadata.py => test_collection_post_metadata.py} |   2 +
 ...tipart.py => test_collection_post_multipart.py} |   2 +
 .../api/{test_deposit_delete.py => test_delete.py} |   8 +-
 .../api/test_deposit_private_read_metadata.py      |   4 +-
 swh/deposit/tests/api/test_deposit_update.py       |  43 ++-
 .../{test_deposit_content.py => test_get_file.py}  |   2 +
 swh/deposit/tests/conftest.py                      |   4 +-
 26 files changed, 610 insertions(+), 557 deletions(-)
 rename swh/deposit/api/{deposit.py => collection.py} (91%)
 rename swh/deposit/api/{deposit_content.py => content.py} (81%)
 delete mode 100644 swh/deposit/api/deposit_update.py
 create mode 100644 swh/deposit/api/edit.py
 create mode 100644 swh/deposit/api/edit_media.py
 rename swh/deposit/api/{deposit_status.py => state.py} (83%)
 create mode 100644 swh/deposit/api/sword_edit.py
 rename swh/deposit/tests/api/{test_deposit.py => test_collection.py} (98%)
 rename swh/deposit/tests/api/{test_deposit_atom.py => test_collection_post_atom.py} (97%)
 rename swh/deposit/tests/api/{test_deposit_binary.py => test_collection_post_binary.py} (97%)
 rename swh/deposit/tests/api/{test_deposit_metadata.py => test_collection_post_metadata.py} (99%)
 rename swh/deposit/tests/api/{test_deposit_multipart.py => test_collection_post_multipart.py} (99%)
 rename swh/deposit/tests/api/{test_deposit_delete.py => test_delete.py} (94%)
 rename swh/deposit/tests/api/{test_deposit_content.py => test_get_file.py} (98%)
Changes applied before test
commit 00b3af02028e548bdbf52346a929671692abbdb9
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Wed Nov 18 14:15:34 2020 +0100

    Remove return values of process_put and process_delete.
    
    They were only used for the special "error" key, which no longer
    exists, so they are useless now.

commit b80d183f1efcdca414b2792bda1fd0b26d597300
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Wed Nov 18 14:07:35 2020 +0100

    Use exceptions instead of a special "error" key in returned dicts.

commit 7a8e6b0abe7a457cb87bd12f7fb469e811447fbe
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Wed Nov 18 13:36:32 2020 +0100

    Generalize BadRequestError to be a DepositError.
    
    First step toward using exceptions instead of dicts everywhere.

commit 66eb322d5c262157470cc4fcac7d696e3668ff9d
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Wed Nov 18 13:19:21 2020 +0100

    Move _compute_md5 to toplevel and remove its docstring.
    
    It doesn't need to be in a class, and the docstring is redundant.

commit 333f9847629bface8f8540d15d22be3a44c8240b
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Wed Nov 18 13:10:07 2020 +0100

    Move Deposit Receipt creation to its own function.
    
    And move _make_iri's code inside this function, it was almost a single statement.

commit 466a0f27d77c4601fc9933b6f67033de4df51cfe
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Wed Nov 18 12:47:00 2020 +0100

    Use an attr class instead of a dict to store parsed headers.
    
    Better typing.

commit 01cb24d3c44059839d0a1daa9c1947426171c019
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Wed Nov 18 11:48:40 2020 +0100

    Split SE-IRI and Edit-IRI.
    
    SWORD doesn't require them to be the same, and IMO it is clearer
    if we keep them separate.

commit 64371a3dc2133a977244e884a191ad7f0162557c
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Wed Nov 18 11:48:35 2020 +0100

    remove assumption that Edit-IRI and SE-IRI are the same from test_post_deposit_atom_entry_multiple_steps.
    
    Currently they are, but a future commit will make them different.

commit 329a2a3785ad8431fa7ec73fee31e86dbee36911
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Wed Nov 18 11:10:49 2020 +0100

    Rename files and classes in swh/deposit/api/deposit_* to be consistent with SWORD terminology.

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

Build is green

Patch application report for D4507 (id=15987)

Could not rebase; Attempt merge onto a67ed6b06b...

Updating a67ed6b0..1f0b39e2
Fast-forward
 swh/deposit/api/{deposit.py => collection.py}      |  12 +-
 swh/deposit/api/common.py                          | 316 ++++++++++-----------
 swh/deposit/api/{deposit_content.py => content.py} |  16 +-
 swh/deposit/api/deposit_update.py                  | 277 ------------------
 swh/deposit/api/edit.py                            | 139 +++++++++
 swh/deposit/api/edit_media.py                      |  96 +++++++
 swh/deposit/api/private/__init__.py                |   8 +-
 swh/deposit/api/private/deposit_update_status.py   |  26 +-
 swh/deposit/api/service_document.py                |   2 +-
 swh/deposit/api/{deposit_status.py => state.py}    |  10 +-
 swh/deposit/api/sword_edit.py                      |  82 ++++++
 swh/deposit/api/urls.py                            |  44 +--
 swh/deposit/config.py                              |   3 +-
 swh/deposit/errors.py                              |  35 ++-
 swh/deposit/settings/common.py                     |   1 +
 swh/deposit/templates/deposit/deposit_receipt.xml  |   4 +-
 .../api/{test_deposit.py => test_collection.py}    |   4 +-
 ...eposit_atom.py => test_collection_post_atom.py} |  11 +-
 ...it_binary.py => test_collection_post_binary.py} |  17 +-
 ...etadata.py => test_collection_post_metadata.py} |   2 +
 ...tipart.py => test_collection_post_multipart.py} |   2 +
 .../api/{test_deposit_delete.py => test_delete.py} |   8 +-
 .../api/test_deposit_private_read_metadata.py      |   4 +-
 swh/deposit/tests/api/test_deposit_update.py       |  43 ++-
 .../{test_deposit_content.py => test_get_file.py}  |   2 +
 swh/deposit/tests/conftest.py                      |   4 +-
 26 files changed, 610 insertions(+), 558 deletions(-)
 rename swh/deposit/api/{deposit.py => collection.py} (91%)
 rename swh/deposit/api/{deposit_content.py => content.py} (81%)
 delete mode 100644 swh/deposit/api/deposit_update.py
 create mode 100644 swh/deposit/api/edit.py
 create mode 100644 swh/deposit/api/edit_media.py
 rename swh/deposit/api/{deposit_status.py => state.py} (83%)
 create mode 100644 swh/deposit/api/sword_edit.py
 rename swh/deposit/tests/api/{test_deposit.py => test_collection.py} (98%)
 rename swh/deposit/tests/api/{test_deposit_atom.py => test_collection_post_atom.py} (97%)
 rename swh/deposit/tests/api/{test_deposit_binary.py => test_collection_post_binary.py} (97%)
 rename swh/deposit/tests/api/{test_deposit_metadata.py => test_collection_post_metadata.py} (99%)
 rename swh/deposit/tests/api/{test_deposit_multipart.py => test_collection_post_multipart.py} (99%)
 rename swh/deposit/tests/api/{test_deposit_delete.py => test_delete.py} (94%)
 rename swh/deposit/tests/api/{test_deposit_content.py => test_get_file.py} (98%)
Changes applied before test
commit 1f0b39e2ed97d5d6661df7301dda1f0efecb3214
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Wed Nov 18 14:15:34 2020 +0100

    Remove return values of process_put and process_delete.
    
    They were only used for the special "error" key, which no longer
    exists, so they are useless now.

commit 229348f845ba99f05a7db1b969a95916e8093d85
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Wed Nov 18 14:07:35 2020 +0100

    Use exceptions instead of a special "error" key in returned dicts.

commit 7a8e6b0abe7a457cb87bd12f7fb469e811447fbe
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Wed Nov 18 13:36:32 2020 +0100

    Generalize BadRequestError to be a DepositError.
    
    First step toward using exceptions instead of dicts everywhere.

commit 66eb322d5c262157470cc4fcac7d696e3668ff9d
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Wed Nov 18 13:19:21 2020 +0100

    Move _compute_md5 to toplevel and remove its docstring.
    
    It doesn't need to be in a class, and the docstring is redundant.

commit 333f9847629bface8f8540d15d22be3a44c8240b
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Wed Nov 18 13:10:07 2020 +0100

    Move Deposit Receipt creation to its own function.
    
    And move _make_iri's code inside this function, it was almost a single statement.

commit 466a0f27d77c4601fc9933b6f67033de4df51cfe
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Wed Nov 18 12:47:00 2020 +0100

    Use an attr class instead of a dict to store parsed headers.
    
    Better typing.

commit 01cb24d3c44059839d0a1daa9c1947426171c019
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Wed Nov 18 11:48:40 2020 +0100

    Split SE-IRI and Edit-IRI.
    
    SWORD doesn't require them to be the same, and IMO it is clearer
    if we keep them separate.

commit 64371a3dc2133a977244e884a191ad7f0162557c
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Wed Nov 18 11:48:35 2020 +0100

    remove assumption that Edit-IRI and SE-IRI are the same from test_post_deposit_atom_entry_multiple_steps.
    
    Currently they are, but a future commit will make them different.

commit 329a2a3785ad8431fa7ec73fee31e86dbee36911
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Wed Nov 18 11:10:49 2020 +0100

    Rename files and classes in swh/deposit/api/deposit_* to be consistent with SWORD terminology.

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

ardumont added a subscriber: ardumont.

I'm fine with the delete part.

Not with put part.

I recall we need to return something, don't we?

swh/deposit/api/edit.py
76

hmm?

What do you do of the multipart_upload result?

86

Is that a tab that got inserted?

This revision now requires changes to proceed.Nov 18 2020, 3:03 PM
swh/deposit/api/edit.py
92

same question about the _atom_entry result.

Oh and i think that breaks the cli part (e.g. for the --replace flag, which does updates).

It's not caught by tests because the requests are mocked with the requests_mock_datadir fixture.

swh/deposit/api/edit.py
76

nothing. it's not used.

$ grep "process_put(" -r swh/**/*.py
swh/deposit/api/common.py:        self.process_put(request, headers, collection_name, deposit_id)
swh/deposit/api/common.py:    def process_put(
swh/deposit/api/edit.py:    def process_put(
swh/deposit/api/edit_media.py:    def process_put(
swh/deposit/api/private/deposit_update_status.py:    def process_put(
86

I added an else, because it shouldn't run if request.content_type.startswith("multipart/")

92

also not used

Oh and i think that breaks the cli part (e.g. for the --replace flag, which does updates).

It's not caught by tests because the requests are mocked with the requests_mock_datadir fixture.

The server does not use the return value of process_put to build its response to the client. If it it, it would be caught in server tests.

The server does not use the return value of process_put to build its response to the client. If it it, it would be caught in server tests.

yes, it seems i misremembered here.

This revision is now accepted and ready to land.Nov 18 2020, 3:37 PM