Details
- Reviewers
anlambert - Group Reviewers
Reviewers - Maniphest Tasks
- T2310: Make origin visits immutable
- Commits
- rDWAPPSa6d6cb345951: service: Use latest origin visit status from an origin
tox
Diff Detail
- Repository
- rDWAPPS Web applications
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
Event Timeline
Build has FAILED
Patch application report for D3316 (id=11736)
Could not rebase; Attempt merge onto c435bc30e4...
Merge made by the 'recursive' strategy. requirements-swh.txt | 2 +- swh/web/common/service.py | 18 +++++++++++++----- swh/web/tests/api/views/test_origin.py | 14 ++++++++++++-- swh/web/tests/conftest.py | 7 ++++--- swh/web/tests/data.py | 3 ++- swh/web/tests/strategies.py | 17 +++++++++-------- 6 files changed, 41 insertions(+), 20 deletions(-)
Changes applied before test
commit f806198f1c07c8fe87f217ac3e829514cd9efb4e
Merge: c435bc30 337c9af5
Author: Jenkins user <jenkins@localhost>
Date: Thu Jun 18 16:44:36 2020 +0000
Merge branch 'diff-target' into HEAD
commit 337c9af5a553835157fefa6d0ebefd291fcfa807
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date: Thu Jun 18 18:42:33 2020 +0200
service: Use latest origin visit status from an origin
Related to T2310
commit ec9d66110224433a7e0193636651a9f60260bbd4
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date: Thu Jun 18 17:37:26 2020 +0200
Migrate to swh.storage.algos.snapshot_get_latest
Related to D3314Link to build: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/192/
See console output for more information: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/192/console
Build has FAILED
Patch application report for D3316 (id=11743)
Could not rebase; Attempt merge onto 0c7cbf061d...
Merge made by the 'recursive' strategy. requirements-swh.txt | 2 +- swh/web/common/service.py | 20 +++++++++++++++----- swh/web/tests/api/views/test_origin.py | 14 ++++++++++++-- swh/web/tests/conftest.py | 7 ++++--- swh/web/tests/data.py | 3 ++- swh/web/tests/strategies.py | 17 +++++++++-------- 6 files changed, 43 insertions(+), 20 deletions(-)
Changes applied before test
commit fe844760abfcb119bd81e2236f1bed1acedd2d69
Merge: 0c7cbf06 a8d64675
Author: Jenkins user <jenkins@localhost>
Date: Fri Jun 19 08:03:22 2020 +0000
Merge branch 'diff-target' into HEAD
commit a8d646754d92691b9e7f43a546d40ea820c3e78a
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date: Thu Jun 18 18:42:33 2020 +0200
service: Use latest origin visit status from an origin
Related to T2310
commit d606a719a398d8bb512070dfebe7aeeffbeee2f0
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date: Thu Jun 18 17:37:26 2020 +0200
Migrate to swh.storage.algos.snapshot_get_latest
Related to D3314Link to build: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/196/
See console output for more information: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/196/console
Looks good, I added some nitpick comments.
| swh/web/common/service.py | ||
|---|---|---|
| 928–929 | How about that instead ? visit, visit_status = visit_and_status if visit_status else ({}, {}) visit_info_dict = {**visit.to_dict(), **visit_status.to_dict()} | |
| swh/web/tests/api/views/test_origin.py | ||
| 368–371 | Maybe you could add a method named origin_visit_get_latest_status in the _ArchiveData class from conftest.py, wrapping the merge of the two dicts returned by swh.storage.algos.origin.origin_get_latest_visit_status ? | |
| swh/web/common/service.py | ||
|---|---|---|
| 928–929 | Fix typo in my previous comment visit, visit_status = visit_and_status if visit_and_status else ({}, {}) visit_info_dict = {**visit.to_dict(), **visit_status.to_dict()} | |
| swh/web/common/service.py | ||
|---|---|---|
| 928–929 |
That i did not thought of
yes, i used that initially but we are only interested by the type visit. The other fields are either redundant (visit, origin) or soon to be removed (status, snapshot, metadata). :) Although, if the issue is the multiple return statements, yes, i gather there is no choice but this then. | |
| swh/web/tests/api/views/test_origin.py | ||
| 368–371 | I'll see what i can do ;) | |
| swh/web/common/service.py | ||
|---|---|---|
| 928–929 | ah also, that won't work. | |
| swh/web/common/service.py | ||
|---|---|---|
| 928–929 | Use this then: visit, visit_status = ( {visit_and_status[0].to_dict(), visit_and_status[1].to_dict()} if visit_and_status else ({}, {}) ) | |
| 928–929 | Typo day ;-) visit, visit_status = ( (visit_and_status[0].to_dict(), visit_and_status[1].to_dict()) if visit_and_status else ({}, {}) ) | |
Build has FAILED
Link to build: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/204/
See console output for more information: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/204/console
| swh/web/common/service.py | ||
|---|---|---|
| 907 | I got mislead by the type here. It should be Optional so a NotFoundExc is raised and properly dealt with in the If we return the empty dict, the test about the not found use case raises a [1] Those are not easy to see when hypothesis tests fail... we got a huge stacktrace about flaky dataset. I got finally out of this one using pdb ¯\_(ツ)_/¯ | |
| 928–929 |
lol ;) | |
| swh/web/common/service.py | ||
|---|---|---|
| 907 | yes, hypothesis makes it hard to debug tests. Passing --hypothesis-verbosity=verbose to pytest usually helps. | |
Build is green
Patch application report for D3316 (id=11751)
Rebasing onto d3ad49fe39...
Current branch diff-target is up to date.
Changes applied before test
commit a6d6cb345951e583d27fde95cddca4e1feb1a121
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date: Thu Jun 18 18:42:33 2020 +0200
service: Use latest origin visit status from an origin
Related to T2310See https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/205/ for more details.