Page MenuHomeSoftware Heritage

loader: Expect visit_date as an optional date in constructors
ClosedPublic

Authored by ardumont on Feb 15 2021, 6:00 PM.

Details

Summary

This avoids side-effect in the constructor and also aligns how it's done in other
loaders (e.g git).

Depends on D5075
Related to T1410

Diff Detail

Repository
rDLDSVN Subversion (SVN) loader
Branch
improve-instantiation
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 19221
Build 29808: Phabricator diff pipeline on jenkinsJenkins console · Jenkins
Build 29807: arc lint + arc unit

Event Timeline

Build has FAILED

Patch application report for D5076 (id=18117)

Could not rebase; Attempt merge onto 71e3b44784...

Updating 71e3b44..78930b4
Fast-forward
 mypy.ini                            |   5 +-
 requirements.txt                    |   1 +
 swh/loader/svn/loader.py            | 111 ++++++++++++++++++++----------------
 swh/loader/svn/tasks.py             |  93 +++++++++++++++++++++---------
 swh/loader/svn/tests/conftest.py    |  44 +++++++-------
 swh/loader/svn/tests/test_loader.py |  92 ++++++++++++++++--------------
 6 files changed, 206 insertions(+), 140 deletions(-)
Changes applied before test
commit 78930b4fca5dbcf428e0ac0826dca0e4bc3ef086
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Sun Feb 7 08:55:36 2021 +0100

    loader: Expect visit_date as an optional date in constructors
    
    This avoids side-effect in the constructor and also aligns how it's done in other
    loaders (e.g git).

commit 727189d56849edd8481db1d6d4b7919c1e935aff
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Sun Feb 7 08:45:16 2021 +0100

    Rework loader instantiation logic according to loader core api
    
    Note that this also updated some docstrings and type along the way.

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

Harbormaster returned this revision to the author for changes because remote builds failed.Feb 15 2021, 6:01 PM
Harbormaster failed remote builds in B19221: Diff 18117!

Build has FAILED

Patch application report for D5076 (id=18121)

Could not rebase; Attempt merge onto 71e3b44784...

Updating 71e3b44..22ad2dd
Fast-forward
 mypy.ini                            |   5 +-
 requirements-swh.txt                |   2 +-
 requirements.txt                    |   1 +
 swh/loader/svn/loader.py            | 111 ++++++++++++++++++++----------------
 swh/loader/svn/tasks.py             |  93 +++++++++++++++++++++---------
 swh/loader/svn/tests/conftest.py    |  44 +++++++-------
 swh/loader/svn/tests/test_loader.py |  92 ++++++++++++++++--------------
 7 files changed, 207 insertions(+), 141 deletions(-)
Changes applied before test
commit 22ad2ddae2bc1d19d7127a5955c6ae0adb3fd56d
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Sun Feb 7 08:55:36 2021 +0100

    loader: Expect visit_date as an optional date in constructors
    
    This avoids side-effect in the constructor and also aligns how it's done in other
    loaders (e.g git).
    
    Related to T1410

commit 69b218f8e22298d21d39b11893c3045bb6cedec8
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Sun Feb 7 08:45:16 2021 +0100

    Rework loader instantiation logic according to loader core api
    
    Note that this also updated some docstrings and type along the way.
    
    Related to T1410

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

Harbormaster returned this revision to the author for changes because remote builds failed.Feb 15 2021, 6:04 PM
Harbormaster failed remote builds in B19225: Diff 18121!

Build has FAILED

Patch application report for D5076 (id=18183)

Could not rebase; Attempt merge onto 71e3b44784...

Updating 71e3b44..2dd86f5
Fast-forward
 mypy.ini                            |   5 +-
 requirements-swh.txt                |   2 +-
 requirements.txt                    |   1 +
 swh/loader/svn/loader.py            | 123 ++++++++++++++++++++----------------
 swh/loader/svn/tasks.py             |  93 +++++++++++++++++++--------
 swh/loader/svn/tests/conftest.py    |  44 +++++++------
 swh/loader/svn/tests/test_loader.py |  92 ++++++++++++++-------------
 7 files changed, 213 insertions(+), 147 deletions(-)
Changes applied before test
commit 2dd86f5e101432242ffdbdd1c2c1cf37dc8de317
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Sun Feb 7 08:55:36 2021 +0100

    loader: Expect visit_date as an optional date in constructors
    
    This avoids side-effect in the constructor and also aligns how it's done in other
    loaders (e.g git).
    
    Related to T1410

commit 5ad2709b12881a8b8a65488a82d7dfc5a0cbb48c
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Sun Feb 7 08:45:16 2021 +0100

    Rework loader instantiation logic according to loader core api
    
    Note that this also updated some docstrings and type along the way.
    
    Related to T1410

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

Harbormaster returned this revision to the author for changes because remote builds failed.Feb 17 2021, 1:09 PM
Harbormaster failed remote builds in B19284: Diff 18183!

Build is green

Patch application report for D5076 (id=18183)

Could not rebase; Attempt merge onto 71e3b44784...

Updating 71e3b44..2dd86f5
Fast-forward
 mypy.ini                            |   5 +-
 requirements-swh.txt                |   2 +-
 requirements.txt                    |   1 +
 swh/loader/svn/loader.py            | 123 ++++++++++++++++++++----------------
 swh/loader/svn/tasks.py             |  93 +++++++++++++++++++--------
 swh/loader/svn/tests/conftest.py    |  44 +++++++------
 swh/loader/svn/tests/test_loader.py |  92 ++++++++++++++-------------
 7 files changed, 213 insertions(+), 147 deletions(-)
Changes applied before test
commit 2dd86f5e101432242ffdbdd1c2c1cf37dc8de317
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Sun Feb 7 08:55:36 2021 +0100

    loader: Expect visit_date as an optional date in constructors
    
    This avoids side-effect in the constructor and also aligns how it's done in other
    loaders (e.g git).
    
    Related to T1410

commit 5ad2709b12881a8b8a65488a82d7dfc5a0cbb48c
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Sun Feb 7 08:45:16 2021 +0100

    Rework loader instantiation logic according to loader core api
    
    Note that this also updated some docstrings and type along the way.
    
    Related to T1410

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

Looks good to me !

swh/loader/svn/tasks.py
15–16

I guess this can be removed.

This revision is now accepted and ready to land.Feb 17 2021, 5:47 PM
swh/loader/svn/tasks.py
15–16

i initially thought "no"

Then i realized you probably mean, it'd be trapped by the try except...

so maybe ;)

Rebase
Drop the unnecessary conditional on None values

Build is green

Patch application report for D5076 (id=18211)

Could not rebase; Attempt merge onto 71e3b44784...

Updating 71e3b44..eead508
Fast-forward
 mypy.ini                            |   5 +-
 requirements-swh.txt                |   2 +-
 requirements.txt                    |   1 +
 swh/loader/svn/loader.py            | 126 ++++++++++++++++++++----------------
 swh/loader/svn/tasks.py             |  91 ++++++++++++++++++--------
 swh/loader/svn/tests/conftest.py    |  44 +++++++------
 swh/loader/svn/tests/test_loader.py |  92 ++++++++++++++------------
 7 files changed, 213 insertions(+), 148 deletions(-)
Changes applied before test
commit eead508f0afe2d4793fbc2199c6832008f889104
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Sun Feb 7 08:55:36 2021 +0100

    loader: Expect visit_date as an optional date in constructors
    
    This avoids side-effect in the constructor and also aligns how it's done in other
    loaders (e.g git).
    
    Related to T1410

commit 2c54129bcf4864537d8d13d6645ef777fc1186ef
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Sun Feb 7 08:45:16 2021 +0100

    Rework loader instantiation logic according to loader core api
    
    Note that this also updated some docstrings and type along the way.
    
    Related to T1410

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