Page MenuHomeSoftware Heritage

loader: Expect visit_date as an optional date in constructors
ClosedPublic

Authored by ardumont on Mon, Feb 15, 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
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 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.Mon, Feb 15, 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.Mon, Feb 15, 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.Wed, Feb 17, 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.Wed, Feb 17, 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.