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

Repository
rDSTO Storage manager
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

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
83

Iterator[Optional[Dict]] is less verbose.

87

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

I think this one will currently make the build fail.

461

never mind me, sorry ;)

ardumont added inline comments.Jun 4 2020, 11:26 AM
swh/storage/cassandra/storage.py
493

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

You can drop the brackets here.

493

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 11421

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/cassandra/storage.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