Page MenuHomeSoftware Heritage
Differential D3218

Adapt to swh.model 0.3
ClosedPublic
Actions

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

Details

Reviewers
ardumont
Group Reviewers
Reviewers
Commits
rDSTOad9c9bbfd0d5: Adapt to swh.model 0.3
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.

Diff Detail

Unit TestsFailed
View All

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

douardda created this revision.Jun 4 2020, 10:49 AM
Herald added a reviewer: Reviewers. · View Herald TranscriptJun 4 2020, 10:49 AM
douardda updated this revision to Diff 11413.Jun 4 2020, 10:53 AM

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

ardumont added a subscriber: ardumont.Jun 4 2020, 10:55 AM
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 ...
swh-public-ci added a comment.Jun 4 2020, 10:56 AM

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

Harbormaster failed remote builds in B12615: Diff 11412!Jun 4 2020, 10:56 AM
swh-public-ci added a comment.Jun 4 2020, 11:02 AM

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

Harbormaster failed remote builds in B12616: Diff 11413!Jun 4 2020, 11:02 AM
ardumont added a comment.Jun 4 2020, 11:03 AM

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.

douardda added a comment.Jun 4 2020, 11:04 AM
In D3218#78434, @ardumont wrote:

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'...

ardumont added a comment.EditedJun 4 2020, 11:08 AM

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 ;)

douardda added a comment.Jun 4 2020, 11:09 AM
In D3218#78437, @ardumont wrote:

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

douardda updated this revision to Diff 11414.Jun 4 2020, 11:20 AM

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

ardumont added inline comments.Jun 4 2020, 11:23 AM
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 ;)

ardumont added inline comments.Jun 4 2020, 11:26 AM
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)

swh-public-ci added a comment.Jun 4 2020, 11:27 AM

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

Harbormaster failed remote builds in B12617: Diff 11414!Jun 4 2020, 11:27 AM
douardda mentioned this in D3203: Replace MockedJournalClient and MockedKafkaWriter by proper kafka test scaffolding.Jun 4 2020, 11:27 AM
douardda updated this revision to Diff 11416.Jun 4 2020, 11:35 AM

another fix for cassandra

ardumont accepted this revision.Jun 4 2020, 11:36 AM

Thanks!

This revision is now accepted and ready to land.Jun 4 2020, 11:36 AM
ardumont mentioned this in D3217: loader-core: Adapt build_revision implementation according to latest model api.Jun 4 2020, 11:40 AM
swh-public-ci added a comment.Jun 4 2020, 11:41 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.

Harbormaster completed remote builds in B12619: Diff 11416.Jun 4 2020, 11:41 AM
olasd added a subscriber: olasd.Jun 4 2020, 11:44 AM
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.

douardda updated this revision to Diff 11419.Jun 4 2020, 11:48 AM

thx olasd

swh-public-ci added a comment.Jun 4 2020, 11:55 AM

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.

Harbormaster completed remote builds in B12623: Diff 11419.Jun 4 2020, 11:55 AM
Closed by commit rDSTOad9c9bbfd0d5: Adapt to swh.model 0.3 (authored by douardda). · Explain WhyJun 4 2020, 12:03 PM
This revision was automatically updated to reflect the committed changes.
douardda added a commit: rDSTOad9c9bbfd0d5: Adapt to swh.model 0.3.

Revision Contents
Changeset List

Diff 11412

View Options

requirements-swh-journal.txt

Loading...
View Options

requirements-swh.txt

Loading...
View Options

swh/storage/cassandra/converters.py

Loading...
View Options

swh/storage/tests/storage_data.py

Loading...
View Options

swh/storage/tests/test_api_client_dicts.py

Loading...
View Options

swh/storage/tests/test_replay.py

Loading...
View Options

swh/storage/tests/test_storage.py

Loading...
View Options

swh/storage/validate.py

Loading...
About · Developer information · Legal