Page MenuHomeSoftware Heritage
Differential D3327

Refactor the Storage.add_origin API
ClosedPublic
Actions

Authored by douardda on Jun 22 2020, 11:37 AM.

Details

Reviewers
vlorentz
Group Reviewers
Reviewers
Commits
rDSTOfb603e1b9874: Make Storage.add_origin() return a sumary dict
Summary

make it consistent with other add_xxx methods, ie. make it return a
summary dict.

Test Plan

Tests of all dependencies (but swh-web, see D3326) should pass OK.

Diff Detail

Unit TestsFailed
View All

TimeTest
517 msJenkins > .tox.py3.lib.python3.7.site-packages.swh.storage.tests.test_cassandra.TestCassandraStorage::test_origin_add
self = <swh.storage.tests.test_cassandra.TestCassandraStorage object at 0x7f84106dc748> swh_storage = <swh.storage.validate.ValidatingProxyStorage object at 0x7f84106e06d8>
644 msJenkins > .tox.py3.lib.python3.7.site-packages.swh.storage.tests.test_cassandra.TestCassandraStorage::test_origin_add
self = <swh.storage.tests.test_cassandra.TestCassandraStorage object at 0x7f016c7a8cc0> swh_storage = <swh.storage.validate.ValidatingProxyStorage object at 0x7f018c611b38>
485 msJenkins > .tox.py3.lib.python3.7.site-packages.swh.storage.tests.test_cassandra.TestCassandraStorage::test_origin_add_from_generator
self = <swh.storage.tests.test_cassandra.TestCassandraStorage object at 0x7f8450615940> swh_storage = <swh.storage.validate.ValidatingProxyStorage object at 0x7f84503fe898>
643 msJenkins > .tox.py3.lib.python3.7.site-packages.swh.storage.tests.test_cassandra.TestCassandraStorage::test_origin_add_from_generator
self = <swh.storage.tests.test_cassandra.TestCassandraStorage object at 0x7f018c60aef0> swh_storage = <swh.storage.validate.ValidatingProxyStorage object at 0x7f01b004d518>
493 msJenkins > .tox.py3.lib.python3.7.site-packages.swh.storage.tests.test_cassandra.TestCassandraStorage::test_origin_add_twice
self = <swh.storage.tests.test_cassandra.TestCassandraStorage object at 0x7f84343954a8> swh_storage = <swh.storage.validate.ValidatingProxyStorage object at 0x7f841057f048>
View Full Test Results (43 Failed · 1,453 Passed · 34 Skipped)

Event Timeline

douardda created this revision.Jun 22 2020, 11:37 AM
Herald added a reviewer: Reviewers. · View Herald TranscriptJun 22 2020, 11:37 AM
douardda added a child revision: D3328: Deprecate the origin_add_one() endpoint.Jun 22 2020, 11:38 AM
swh-public-ci added a comment.Jun 22 2020, 11:45 AM

Build has FAILED

Patch application report for D3327 (id=11786)

Rebasing onto 041543df09...

First, rewinding head to replay your work on top of it...
Applying: Refactor the Storage.add_origin API
Changes applied before test
commit 5bb22b8d6b629cba0a86d574d7e9748e17b5232c
Author: David Douard <david.douard@sdfa3.org>
Date:   Thu Jun 18 18:28:51 2020 +0200

    Refactor the Storage.add_origin API
    
    make it consistent with other add_xxx methods, ie. make it return a
    summary dict.

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

Harbormaster failed remote builds in B12998: Diff 11786!Jun 22 2020, 11:45 AM
douardda updated this revision to Diff 11796.Jun 22 2020, 2:39 PM

Fix for cassandra

swh-public-ci added a comment.Jun 22 2020, 2:46 PM

Build has FAILED

Patch application report for D3327 (id=11796)

Rebasing onto 041543df09...

First, rewinding head to replay your work on top of it...
Applying: Refactor the Storage.add_origin API
Changes applied before test
commit 4a8e2cf9d048e6939ca8fa37f1a632c7b876b2c2
Author: David Douard <david.douard@sdfa3.org>
Date:   Thu Jun 18 18:28:51 2020 +0200

    Refactor the Storage.add_origin API
    
    make it consistent with other add_xxx methods, ie. make it return a
    summary dict.

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

Harbormaster failed remote builds in B13007: Diff 11796!Jun 22 2020, 2:46 PM
swh-public-ci added a comment.Jun 22 2020, 3:24 PM

Build has FAILED

Patch application report for D3327 (id=11796)

Rebasing onto 041543df09...

First, rewinding head to replay your work on top of it...
Applying: Refactor the Storage.add_origin API
Changes applied before test
commit b666b60209ddac2a000d5e97362f3b3df932466f
Author: David Douard <david.douard@sdfa3.org>
Date:   Thu Jun 18 18:28:51 2020 +0200

    Refactor the Storage.add_origin API
    
    make it consistent with other add_xxx methods, ie. make it return a
    summary dict.

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

vlorentz requested changes to this revision.Jun 22 2020, 4:37 PM
vlorentz added a subscriber: vlorentz.

Could you change the first line of your commit message to explain what it changed?

swh/storage/cassandra/storage.py
767

It is, but it doesn't matter. If an origin gets inserted twice at the same time, both entries have the same primary key, so only the latter is kept.

768–772

Remove the brackets around the call to origin_get

This revision now requires changes to proceed.Jun 22 2020, 4:37 PM
douardda added inline comments.Jun 22 2020, 4:44 PM
swh/storage/cassandra/storage.py
767

ok thx

768–772

thx again

douardda added inline comments.Jun 22 2020, 4:55 PM
swh/storage/cassandra/storage.py
767

Now I remember: what may be inconsistent is the number of origin added retruned in the status dict {"origin:add": n} but I guess it's not really a problem.

vlorentz added inline comments.Jun 22 2020, 4:57 PM
swh/storage/cassandra/storage.py
767

yup.

all stats are approximates anyway

douardda updated this revision to Diff 11813.Jun 22 2020, 5:05 PM

reword the commit message and fix cassandra impl

swh-public-ci added a comment.Jun 22 2020, 5:12 PM

Build has FAILED

Patch application report for D3327 (id=11813)

Rebasing onto 2d497ff07c...

First, rewinding head to replay your work on top of it...
Applying: Make Storage.add_origin() return a sumary dict
Changes applied before test
commit 935e5c71b1bdc72805ea69ca569a7bca5af800f6
Author: David Douard <david.douard@sdfa3.org>
Date:   Thu Jun 18 18:28:51 2020 +0200

    Make Storage.add_origin() return a sumary dict
    
    make it consistent with other add_xxx methods by making it return a
    summary dict `{"origin:add": int}`.

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

Harbormaster failed remote builds in B13028: Diff 11813!Jun 22 2020, 5:12 PM
vlorentz accepted this revision.Jun 22 2020, 5:42 PM
This revision is now accepted and ready to land.Jun 22 2020, 5:42 PM
ardumont added a subscriber: ardumont.Jun 22 2020, 6:10 PM
ardumont added inline comments.
swh/storage/cassandra/storage.py
768–772

To repeat what i've said on irc, I think that filtering might be probably the root cause of the current tests failing...

douardda updated this revision to Diff 11836.Jun 23 2020, 2:55 PM

add a revision to fix test_origin_visit_status_add_twice

needed for the tests to pass ok with the cassandra backend after
the refactoring.

swh-public-ci added a comment.Jun 23 2020, 3:02 PM

Build has FAILED

Patch application report for D3327 (id=11836)

Rebasing onto 2d497ff07c...

First, rewinding head to replay your work on top of it...
Applying: Simplify/fix test_origin_visit_status_add_twice
Applying: Make Storage.add_origin() return a sumary dict
Changes applied before test
commit 22b602541c9919557cc9c122d8e24912d0be2d56
Author: David Douard <david.douard@sdfa3.org>
Date:   Thu Jun 18 18:28:51 2020 +0200

    Make Storage.add_origin() return a sumary dict
    
    make it consistent with other add_xxx methods by making it return a
    summary dict `{"origin:add": int}`.

commit d706e200687f569c05cceebc8645a8cfde22a9ab
Author: David Douard <david.douard@sdfa3.org>
Date:   Mon Jun 22 17:47:26 2020 +0200

    Simplify/fix test_origin_visit_status_add_twice
    
    do not make assumptions on how the backend storage will manage idempotence
    related to the journal.

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

Harbormaster failed remote builds in B13051: Diff 11836!Jun 23 2020, 3:02 PM
douardda updated this revision to Diff 11840.Jun 23 2020, 4:08 PM

remove the previously added revision and fix the actual bug instead

[...]

swh-public-ci added a comment.Jun 23 2020, 4:15 PM

Build is green

Patch application report for D3327 (id=11840)

Rebasing onto 2d497ff07c...

Current branch diff-target is up to date.
Changes applied before test
commit fb603e1b9874a3bbb393f7653ec5004bfa18b251
Author: David Douard <david.douard@sdfa3.org>
Date:   Thu Jun 18 18:28:51 2020 +0200

    Make Storage.add_origin() return a sumary dict
    
    make it consistent with other add_xxx methods by making it return a
    summary dict `{"origin:add": int}`.

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

Harbormaster completed remote builds in B13055: Diff 11840.Jun 23 2020, 4:15 PM
Closed by commit rDSTOfb603e1b9874: Make Storage.add_origin() return a sumary dict (authored by douardda). · Explain WhyJun 23 2020, 4:49 PM
This revision was automatically updated to reflect the committed changes.
douardda added a commit: rDSTOfb603e1b9874: Make Storage.add_origin() return a sumary dict.

Revision Contents
Changeset List

PathSize
swh/
storage/
cassandra/
16 lines
12 lines
6 lines
34 lines
tests/
algos/
7 lines
82 lines
4 lines

Diff 11796

View Options

swh/storage/cassandra/storage.py

Loading...
View Options

swh/storage/in_memory.py

Loading...
View Options

swh/storage/interface.py

Loading...
View Options

swh/storage/storage.py

Loading...
View Options

swh/storage/tests/algos/test_origin.py

Loading...
View Options

swh/storage/tests/test_storage.py

Loading...
View Options

swh/storage/validate.py

Loading...
About · Developer information · Legal