Page MenuHomeSoftware Heritage

Adapt to swh.model 0.3
ClosedPublic

Authored by douardda on Jun 4 2020, 10:49 AM.

Details

Summary

in which List attributes have replaced by Tuple ones.

This requires a bit of adaptation in the code of the
ValidatingProxyStorage to ensure dict representation of revision
objects are properly typed.

Unit TestsFailed

TimeTest
555 msJenkins > .tox.py3.lib.python3.7.site-packages.swh.storage.tests.test_cassandra.TestCassandraStorage::test_revision_get
@contextlib.contextmanager def convert_validation_exceptions(): """Catches validation errors arguments, and re-raises a
529 msJenkins > .tox.py3.lib.python3.7.site-packages.swh.storage.tests.test_cassandra.TestCassandraStorage::test_revision_get_no_parents
@contextlib.contextmanager def convert_validation_exceptions(): """Catches validation errors arguments, and re-raises a
587 msJenkins > .tox.py3.lib.python3.7.site-packages.swh.storage.tests.test_cassandra.TestCassandraStorage::test_revision_get_order
@contextlib.contextmanager def convert_validation_exceptions(): """Catches validation errors arguments, and re-raises a
616 msJenkins > .tox.py3.lib.python3.7.site-packages.swh.storage.tests.test_cassandra.TestCassandraStorage::test_revision_log
self = <swh.storage.tests.test_cassandra.TestCassandraStorage object at 0x7f1af441fc88> swh_storage = <swh.storage.validate.ValidatingProxyStorage object at 0x7f1b346abba8>
527 msJenkins > .tox.py3.lib.python3.7.site-packages.swh.storage.tests.test_cassandra.TestCassandraStorage::test_revision_log_with_limit
self = <swh.storage.tests.test_cassandra.TestCassandraStorage object at 0x7f1af40fe2b0> swh_storage = <swh.storage.validate.ValidatingProxyStorage object at 0x7f1af40fe358>
View Full Test Results (5 Failed · 733 Passed · 17 Skipped)

Event Timeline

add a word in the commit message about test_api_client_dicts.py removal

ardumont added inline comments.
swh/storage/validate.py
84

Iterator[Optional[Dict]] is less verbose.

88

less characters (and kinda avoid a "seemingly" double negation ;)

if rev_dict is None:
    yield None
else:
    yield ...

Build has FAILED

Patch application report for D3218 (id=11412)

Rebasing onto eef4900db5...

Current branch diff-target is up to date.
Changes applied before test
commit 5e0fa4bc0c1c1038137124f407ee6f9adce6f7c6
Author: David Douard <david.douard@sdfa3.org>
Date:   Tue Jun 2 17:25:18 2020 +0200

    Adapt to swh.model 0.3
    
    in which List attributes have replaced by Tuple ones.
    
    This requires a bit of adaptation in the code of the
    ValidatingProxyStorage to ensure dict representation of revision
    objects are properly typed.

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

Build has FAILED

Patch application report for D3218 (id=11413)

Rebasing onto eef4900db5...

Current branch diff-target is up to date.
Changes applied before test
commit 23ab0fa0bf2adaaa1ee0fef44ddffecc044f2f95
Author: David Douard <david.douard@sdfa3.org>
Date:   Tue Jun 2 17:25:18 2020 +0200

    Adapt to swh.model 0.3
    
    in which List attributes have replaced by Tuple ones.
    
    This requires a bit of adaptation in the code of the
    ValidatingProxyStorage to ensure dict representation of revision
    objects are properly typed.
    
    The test_api_client_dicts.py has been removed since it's not really
    useful any more and would require a fair amount of work to fix it.

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

Build has FAILED

My understanding of the current log failure is that the cassandra returns you
"parents" as a list instead of tuple. This then is captured by the validation
proxy.

Build has FAILED

My understanding of the current log failure is that the cassandra returns you
"parents" as a list instead of tuple. This then is captured by the validation
proxy.

probably, the pb of running tests with 'not cassandra'...

My understanding of the current log failure is that the cassandra returns you
"parents" as a list instead of tuple. This then is captured by the validation
proxy.

probably, the pb of running tests with 'not cassandra'...

yeah ;)

Jsyk, in D3211 (which was missing the rest of your current diff), I took the
party of keeping the list type in the conversion function (you changed it to
Tuple here) cassandra.converters.revision_from_db and force the parents to be
a tuple within that function. This, to avoid having to change the
cassandra.storage implementations. As you took the other path, you need to
check the revision_get and other revision functions which builds list for that field ;)

My understanding of the current log failure is that the cassandra returns you
"parents" as a list instead of tuple. This then is captured by the validation
proxy.

probably, the pb of running tests with 'not cassandra'...

yeah ;)

Jsyk, in D3211 (which was missing the rest of your current diff), I took the
party of keeping the list type in the conversion function (you changed it to
Tuple here) cassandra.converters.revision_from_db and force the parents to be
a tuple within that function. This, to avoid having to change the
cassandra.storage implementations. As you took the other path, you need to
check the revision_get and other revision functions which builds list for that field ;)

thx

adapt according to ardumont's comments and (maybe) fix cassandra tests

swh/storage/cassandra/storage.py
461 ↗(On Diff #11414)

I think this one will currently make the build fail.

461 ↗(On Diff #11414)

never mind me, sorry ;)

swh/storage/cassandra/storage.py
493 ↗(On Diff #11414)

This one could make the build fail... (I don't get why mypy is not complaining with the revision_from_db call)

Build has FAILED

Patch application report for D3218 (id=11414)

Rebasing onto eef4900db5...

Current branch diff-target is up to date.
Changes applied before test
commit d1bcb9c638d671474c4d5b89d5eaa0869d177419
Author: David Douard <david.douard@sdfa3.org>
Date:   Tue Jun 2 17:25:18 2020 +0200

    Adapt to swh.model 0.3
    
    in which List attributes have replaced by Tuple ones.
    
    This requires a bit of adaptation in the code of the
    ValidatingProxyStorage to ensure dict representation of revision
    objects are properly typed.
    
    The test_api_client_dicts.py has been removed since it's not really
    useful any more and would require a fair amount of work to fix it.

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

This revision is now accepted and ready to land.Jun 4 2020, 11:36 AM

Build is green

Patch application report for D3218 (id=11416)

Rebasing onto eef4900db5...

Current branch diff-target is up to date.
Changes applied before test
commit eef8509a4b6ddc9c08cd0fed06486dfa3ac4a7a5
Author: David Douard <david.douard@sdfa3.org>
Date:   Tue Jun 2 17:25:18 2020 +0200

    Adapt to swh.model 0.3
    
    in which List attributes have replaced by Tuple ones.
    
    This requires a bit of adaptation in the code of the
    ValidatingProxyStorage to ensure dict representation of revision
    objects are properly typed.
    
    The test_api_client_dicts.py has been removed since it's not really
    useful any more and would require a fair amount of work to fix it.

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

olasd added inline comments.
swh/storage/cassandra/storage.py
461 ↗(On Diff #11416)

You can drop the brackets here.

493 ↗(On Diff #11416)

Same, you can drop the brackets.

Build is green

Patch application report for D3218 (id=11419)

Rebasing onto eef4900db5...

Current branch diff-target is up to date.
Changes applied before test
commit ad9c9bbfd0d536217e895fd41408c8c11ceafc66
Author: David Douard <david.douard@sdfa3.org>
Date:   Tue Jun 2 17:25:18 2020 +0200

    Adapt to swh.model 0.3
    
    in which List attributes have replaced by Tuple ones.
    
    This requires a bit of adaptation in the code of the
    ValidatingProxyStorage to ensure dict representation of revision
    objects are properly typed.
    
    The test_api_client_dicts.py has been removed since it's not really
    useful any more and would require a fair amount of work to fix it.

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

This revision was automatically updated to reflect the committed changes.