Page MenuHomeSoftware Heritage

Use loader_snapshot_id from swh.loader.core 0.1.0 instead of get_snapshot_id
ClosedPublic

Authored by olasd on May 29 2020, 4:48 PM.

Diff Detail

Repository
rDLDSVN Subversion (SVN) loader
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

Build is green

Patch application report for D3204 (id=11371)

Could not rebase; Attempt merge onto efe36bbfc2...

Updating efe36bb..558f280
Fast-forward
 requirements-swh.txt     | 2 +-
 swh/loader/svn/loader.py | 7 ++-----
 swh/loader/svn/tasks.py  | 2 +-
 3 files changed, 4 insertions(+), 7 deletions(-)
Changes applied before test
commit 558f280a22190c5bdecf3c90138e10714ba5bf24
Author: Nicolas Dandrimont <nicolas@dandrimont.eu>
Date:   Fri May 29 16:46:17 2020 +0200

    Use loader_snapshot_id from swh.loader.core 0.1.0 instead of get_snapshot_id

commit 8b45d5c80a30f7430e8f2b1f312a26b82be77872
Author: Nicolas Dandrimont <nicolas@dandrimont.eu>
Date:   Fri May 29 15:58:26 2020 +0200

    Fix black for py37+

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

ardumont added inline comments.
swh/loader/svn/loader.py
553

self.snapshot could be None...

According to the generate_and_load_snapshot implementation. Thus why the
get_snapshot_id implementation was checking it.

I don't remember if it was to respect fully the type or if it was for a good
reason though. ¯\_(ツ)_/¯

swh/loader/svn/loader.py
553

D2888 introduced this change but it kept the conditional which was there already

This fixes the expected broken ci build [1]

[1] https://jenkins.softwareheritage.org/job/DLDSVN/job/tests/645

(I know you know, it's for others ;)

This revision is now accepted and ready to land.May 29 2020, 5:41 PM
swh/loader/svn/loader.py
553

I'd say calling generate_and_load_snapshot with a None revision and snapshot is a loader bug. I'll send a followup diff to check that.

This revision was landed with ongoing or failed builds.Jun 2 2020, 6:51 PM
This revision was automatically updated to reflect the committed changes.

Build is green

Patch application report for D3204 (id=11384)

Could not rebase; Attempt merge onto dd4e6c4f94...

Updating dd4e6c4..f934581
Fast-forward
 requirements-swh.txt     | 2 +-
 swh/loader/svn/loader.py | 7 ++-----
 swh/loader/svn/tasks.py  | 2 +-
 3 files changed, 4 insertions(+), 7 deletions(-)
Changes applied before test
commit f934581b2015afd351b0db644b6eac87d7703d27
Author: Nicolas Dandrimont <nicolas@dandrimont.eu>
Date:   Fri May 29 16:46:17 2020 +0200

    Use loader_snapshot_id from swh.loader.core 0.1.0 instead of get_snapshot_id

commit 9f9bb80ec1f69d39fb9f1bde2e644f9258dfda2a
Author: Nicolas Dandrimont <nicolas@dandrimont.eu>
Date:   Fri May 29 15:58:26 2020 +0200

    Fix black for py37+

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