Page MenuHomeSoftware Heritage

deposit_check: Actually store warning in deposit status detail
ClosedPublic

Authored by ardumont on Feb 21 2022, 4:00 PM.

Details

Summary

Prior to this commit, only rejected deposit were storing problem details. Now that we
can have warnings even in case of 'verified' deposit, we need to store that details for
post-analysis.

Note that this also fixes the docstring of the overall class which were out of date
since the beginning (duplicated from another class).

Related to T3677
Depends on D7209

Diff Detail

Repository
rDDEP Push deposit
Branch
store-warning
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 27010
Build 42233: Phabricator diff pipeline on jenkinsJenkins console · Jenkins
Build 42232: arc lint + arc unit

Event Timeline

Build has FAILED

Patch application report for D7210 (id=26132)

Could not rebase; Attempt merge onto 40adc8c23b...

Updating 40adc8c2..e338e404
Fast-forward
 swh/deposit/api/checks.py                          | 30 ++++++--
 swh/deposit/api/private/deposit_check.py           | 85 ++++++++++++----------
 swh/deposit/tests/api/test_checks.py               | 80 ++++++++++++++++++--
 .../tests/api/test_deposit_private_check.py        | 21 +++++-
 4 files changed, 161 insertions(+), 55 deletions(-)
Changes applied before test
commit e338e404731998cf75bcb1a86f74b11b6c63d976
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Mon Feb 21 15:56:55 2022 +0100

    deposit_check: Actually store warning in deposit status detail
    
    Prior to this commit, only reject deposit were storing the problem details. Now that we
    can have warning even in case of verified deposit, we need to store that details for
    post-analysis later.
    
    Related to T3677

commit b9749edc15f90a63c12f47e022a92eecf27161e6
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Mon Feb 21 15:26:13 2022 +0100

    api.checks: Warn when suggested fields are missing from metadata
    
    This introduces a new check about the metadata provenance. While it's a suggested field,
    it's definitely something that we want deposit clients to send us. So warn when it's not
    the case. That does not reject the deposit but it's worth keeping that detail in the
    backend.
    
    Related to T3677

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

Harbormaster returned this revision to the author for changes because remote builds failed.Feb 21 2022, 4:04 PM
Harbormaster failed remote builds in B27003: Diff 26132!

Build is green

Patch application report for D7210 (id=26137)

Could not rebase; Attempt merge onto 40adc8c23b...

Updating 40adc8c2..edf0f7ed
Fast-forward
 swh/deposit/api/checks.py                          | 30 ++++++--
 swh/deposit/api/private/deposit_check.py           | 85 ++++++++++++----------
 swh/deposit/tests/api/test_checks.py               | 80 ++++++++++++++++++--
 .../tests/api/test_deposit_private_check.py        | 21 +++++-
 swh/deposit/tests/cli/test_client.py               | 22 +++++-
 5 files changed, 179 insertions(+), 59 deletions(-)
Changes applied before test
commit edf0f7ed8059bfddb454737b5d40b35236cb1c4c
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Mon Feb 21 15:56:55 2022 +0100

    deposit_check: Actually store warning in deposit status detail
    
    Prior to this commit, only reject deposit were storing the problem details. Now that we
    can have warning even in case of verified deposit, we need to store that details for
    post-analysis later.
    
    Related to T3677

commit 339f7dd390ca8cbe8e79a77a1bc2e3c704d5f3f1
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Mon Feb 21 15:26:13 2022 +0100

    api.checks: Warn when suggested fields are missing from metadata
    
    This introduces a new check about the metadata provenance. While it's a suggested field,
    it's definitely something that we want deposit clients to send us. So warn when it's not
    the case. That does not reject the deposit but it's worth keeping that detail in the
    backend.
    
    Related to T3677

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

  • Rework class docstring
  • rework commit message
ardumont added inline comments.
swh/deposit/api/private/deposit_check.py
190

mmm, lies! It's the same behavior as before!!

190

(coverage related ^)

swh/deposit/api/private/deposit_check.py
70

simpler, and matches the output document

157–159

ditto

Build is green

Patch application report for D7210 (id=26139)

Could not rebase; Attempt merge onto 40adc8c23b...

Updating 40adc8c2..62ec72f2
Fast-forward
 swh/deposit/api/checks.py                          | 30 ++++++--
 swh/deposit/api/private/deposit_check.py           | 84 ++++++++++++----------
 swh/deposit/tests/api/test_checks.py               | 80 ++++++++++++++++++---
 .../tests/api/test_deposit_private_check.py        | 21 +++++-
 swh/deposit/tests/cli/test_client.py               | 22 ++++--
 5 files changed, 178 insertions(+), 59 deletions(-)
Changes applied before test
commit 62ec72f2a96c4cc0f8a20c60a25dd886ca22de2f
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Mon Feb 21 15:56:55 2022 +0100

    deposit_check: Actually store warning in deposit status detail
    
    Prior to this commit, only rejected deposit were storing problem details. Now that we
    can have warnings even in case of 'verified' deposit, we need to store that details for
    post-analysis.
    
    Note that this also fixes the docstring of the overall class which were out of date
    since the beginning (duplicated from another class).
    
    Related to T3677

commit 339f7dd390ca8cbe8e79a77a1bc2e3c704d5f3f1
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Mon Feb 21 15:26:13 2022 +0100

    api.checks: Warn when suggested fields are missing from metadata
    
    This introduces a new check about the metadata provenance. While it's a suggested field,
    it's definitely something that we want deposit clients to send us. So warn when it's not
    the case. That does not reject the deposit but it's worth keeping that detail in the
    backend.
    
    Related to T3677

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

swh/deposit/api/private/deposit_check.py
70

awesome ;)

Adapt according to suggestion

Build is green

Patch application report for D7210 (id=26141)

Could not rebase; Attempt merge onto 40adc8c23b...

Updating 40adc8c2..770cc0f5
Fast-forward
 swh/deposit/api/checks.py                          | 30 ++++++--
 swh/deposit/api/private/deposit_check.py           | 80 ++++++++++++----------
 swh/deposit/tests/api/test_checks.py               | 80 +++++++++++++++++++---
 .../tests/api/test_deposit_private_check.py        | 21 +++++-
 swh/deposit/tests/cli/test_client.py               | 22 ++++--
 5 files changed, 174 insertions(+), 59 deletions(-)
Changes applied before test
commit 770cc0f5152d948e32f3f0cf0640f50a17626923
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Mon Feb 21 15:56:55 2022 +0100

    deposit_check: Actually store warning in deposit status detail
    
    Prior to this commit, only rejected deposit were storing problem details. Now that we
    can have warnings even in case of 'verified' deposit, we need to store that details for
    post-analysis.
    
    Note that this also fixes the docstring of the overall class which were out of date
    since the beginning (duplicated from another class).
    
    Related to T3677

commit 339f7dd390ca8cbe8e79a77a1bc2e3c704d5f3f1
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Mon Feb 21 15:26:13 2022 +0100

    api.checks: Warn when suggested fields are missing from metadata
    
    This introduces a new check about the metadata provenance. While it's a suggested field,
    it's definitely something that we want deposit clients to send us. So warn when it's not
    the case. That does not reject the deposit but it's worth keeping that detail in the
    backend.
    
    Related to T3677

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

This revision is now accepted and ready to land.Feb 21 2022, 5:16 PM