Details

Reviewers

vlorentz

Group Reviewers

Reviewers

Maniphest Tasks

T2310: Make origin visits immutable

Commits

rDSTO37c45300bc44: storage*: Align origin-visit-add to take iterable of OriginVisit objects

Summary

This makes its api consistent with other add endpoints (see
origin-visit-status-add as one example).

This is preparatory work towards removing origin-visit-upsert.

It's similar to origin-visit-upsert now but it does slighly more:

it checks for origin existence (raise if unknown)
it's able to either deal with visit with id (replayer case) and visit with no id (the existing use case)

It does also less by delegating the validation to the model instantiation
(prior to the call of the endpoint) which is a good thing.

This also drops the dateutil.parser we want to phase out.

Related to T2310

Test Plan

tox

Diff Detail

Repository

rDSTO Storage manager

Branch

origin-visit-add

Lint

No Linters Available

Unit

No Unit Test Coverage

Build Status

Buildable 12781
Build 19444: Phabricator diff pipeline on jenkins	Jenkins console · Jenkins
Build 19443: arc lint + arc unit

Event Timeline

ardumont created this revision.Jun 11 2020, 3:26 PM

ardumont edited the summary of this revision. (Show Details)Jun 11 2020, 3:27 PM

Fix mispelled comment
Make sure we *mostly* write to the journal prior to the backend when possible (not possible for the pg backend in the case of new origin visit).

Build has FAILED

Patch application report for D3262 (id=11568)

Rebasing onto 5d6163342f...

Current branch diff-target is up to date.

Changes applied before test

commit 2e88faeccc5d6cab044dcc83ab131fd92818a718
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Thu Jun 11 14:57:02 2020 +0200

    storage*: Align origin-visit-add to take iterable of OriginVisit objects
    
    This makes its api consistent with other add endpoints.
    
    This is preparatory work towards removing origin-visit-upsert.
    
    Related to T2310

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

Harbormaster failed remote builds in B12773: Diff 11568!Jun 11 2020, 3:34 PM

ardumont planned changes to this revision.Jun 11 2020, 3:38 PM

Build has FAILED

Patch application report for D3262 (id=11569)

Rebasing onto 5d6163342f...

Current branch diff-target is up to date.

Changes applied before test

commit 73c45b82779da698b3f7f1bcdd1ebd67752fd801
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Thu Jun 11 14:57:02 2020 +0200

    storage*: Align origin-visit-add to take iterable of OriginVisit objects
    
    This makes its api consistent with other add endpoints.
    
    This is preparatory work towards removing origin-visit-upsert.
    
    Related to T2310

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

Harbormaster failed remote builds in B12774: Diff 11569!Jun 11 2020, 3:42 PM

Fix most replayer missing origin-visit-add calls

Expects tests to be a tad red again (replayer wise only) because of the
in-memory storage implementation on passing existing origin-visit... unsure as
how to properly deal with that one yet.

ardumont planned changes to this revision.Jun 11 2020, 4:19 PM

Build has FAILED

Patch application report for D3262 (id=11571)

Rebasing onto 5d6163342f...

Current branch diff-target is up to date.

Changes applied before test

commit d95aeca3d79b59268322959cb058f876d10b93bd
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Thu Jun 11 16:18:01 2020 +0200

    in-memory: wip fix (unsure as what to do for the idempotency case for now)

commit 691cdc54a3fc78e8f281091d36df06b48d1da354
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Thu Jun 11 14:57:02 2020 +0200

    storage*: Align origin-visit-add to take iterable of OriginVisit objects
    
    This makes its api consistent with other add endpoints.
    
    This is preparatory work towards removing origin-visit-upsert.
    
    Related to T2310

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

Harbormaster failed remote builds in B12776: Diff 11571!Jun 11 2020, 4:27 PM

Fix replayer tests

ardumont edited the summary of this revision. (Show Details)Jun 11 2020, 5:11 PM

ardumont retitled this revision from storage*: Align origin-visit-add to take iterable of OriginVisit objects to storage*: Align origin-visit-add to take iterable of OriginVisit objects as input.Jun 11 2020, 5:13 PM

Build is green

Patch application report for D3262 (id=11572)

Rebasing onto 5d6163342f...

Current branch diff-target is up to date.

Changes applied before test

commit ceed32bc9c95e2c997beaeae2f447253291e58cd
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Thu Jun 11 14:57:02 2020 +0200

    storage*: Align origin-visit-add to take iterable of OriginVisit objects
    
    This makes its api consistent with other add endpoints.
    
    This is preparatory work towards removing origin-visit-upsert.
    
    Related to T2310

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

Harbormaster completed remote builds in B12777: Diff 11572.Jun 11 2020, 5:16 PM

ardumont mentioned this in T2310: Make origin visits immutable.Jun 11 2020, 5:46 PM

vlorentz accepted this revision.Jun 11 2020, 6:12 PM

vlorentz added inline comments.

swh/storage/cassandra/storage.py
810	`visit = attr.evolve(visit, visit=visit_id)`
swh/storage/in_memory.py
817–819	same
swh/storage/storage.py
841	same
swh/storage/tests/test_replay.py
73–80	cool
swh/storage/tests/test_storage.py
1623–1633	inconsistent names: `expected_ori_visit_dict` vs `visit_status`
1683–1684	good
1684–1688	nit: `(origin_visit1, origin_visits) = swh_storage.origin_visit_add([visit1, visit2])`
1778–1780	same
1991	may it still raise a ValueError?
1996	dead code

This revision is now accepted and ready to land.Jun 11 2020, 6:12 PM

ardumont added inline comments.Jun 11 2020, 6:22 PM

swh/storage/cassandra/storage.py
810	oh yeah! thanks!
swh/storage/tests/test_storage.py
1623–1633	yeah, i recall it was to avoid too long lines...or something, i'll see what i can do.
1684–1688	right, i forgot to make a pass to check the ones i missed ;)
1991	no idea, i'll check.

ardumont marked 4 inline comments as done.Jun 11 2020, 6:29 PM

ardumont added inline comments.

swh/storage/tests/test_storage.py
1991	it's origin-visit-update here not add so i guess, yes it still raises. Also, note that i will tear it down soon so don't be attached too much ;)

Adapt according to review

ardumont edited the summary of this revision. (Show Details)Jun 11 2020, 6:31 PM

Build is green

Patch application report for D3262 (id=11576)

Rebasing onto 5d6163342f...

Current branch diff-target is up to date.

Changes applied before test

commit 785e480ed1b54fde0a26094c92ea2583a3a95679
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Thu Jun 11 14:57:02 2020 +0200

    storage*: Align origin-visit-add to take iterable of OriginVisit objects
    
    This makes its api consistent with other add endpoints.
    
    This is preparatory work towards removing origin-visit-upsert.
    
    Related to T2310

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

Harbormaster completed remote builds in B12781: Diff 11576.Jun 11 2020, 6:39 PM

ardumont edited the summary of this revision. (Show Details)Jun 11 2020, 6:47 PM

Missed a spot ;)

ardumont added a child revision: D3264: Migrate to new storage.origin_visit_add endpoint.Jun 11 2020, 6:57 PM

ardumont added a child revision: D3265: tests: Migrate to new storage.origin_visit_add endpoint.

Build has FAILED

Patch application report for D3262 (id=11578)

Rebasing onto 5d6163342f...

Current branch diff-target is up to date.

Changes applied before test

commit 37c45300bc44694fddf945bd70540b2ece5e81c3
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Thu Jun 11 14:57:02 2020 +0200

    storage*: Align origin-visit-add to take iterable of OriginVisit objects
    
    This makes its api consistent with other add endpoints.
    
    This is preparatory work towards removing origin-visit-upsert.
    
    Related to T2310

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

Harbormaster failed remote builds in B12783: Diff 11578!Jun 11 2020, 7:02 PM

ardumont added a child revision: D3267: swh-web: Migrate to new storage.origin_visit_add endpoint.Jun 11 2020, 7:11 PM

Build is green

Patch application report for D3262 (id=11578)

Rebasing onto 5d6163342f...

Current branch diff-target is up to date.

Changes applied before test

commit 37c45300bc44694fddf945bd70540b2ece5e81c3
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Thu Jun 11 14:57:02 2020 +0200

    storage*: Align origin-visit-add to take iterable of OriginVisit objects
    
    This makes its api consistent with other add endpoints.
    
    This is preparatory work towards removing origin-visit-upsert.
    
    Related to T2310

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

Harbormaster completed remote builds in B12783: Diff 11578.Jun 11 2020, 7:24 PM

Closed by commit rDSTO37c45300bc44: storage*: Align origin-visit-add to take iterable of OriginVisit objects (authored by ardumont). · Explain WhyJun 12 2020, 9:06 AM

This revision was automatically updated to reflect the committed changes.

ardumont added a commit: rDSTO37c45300bc44: storage*: Align origin-visit-add to take iterable of OriginVisit objects.

storage*: Align origin-visit-add to take iterable of OriginVisit objects as input
ClosedPublic
Actions

Details

Diff Detail

Event Timeline

Patch application report for D3262 (id=11568)

Changes applied before test

Patch application report for D3262 (id=11569)

Changes applied before test

Patch application report for D3262 (id=11571)

Changes applied before test

Patch application report for D3262 (id=11572)

Changes applied before test

Patch application report for D3262 (id=11576)

Changes applied before test

Patch application report for D3262 (id=11578)

Changes applied before test

Patch application report for D3262 (id=11578)

Changes applied before test

Revision Contents
Changeset List

Diff 11576

requirements.txt

swh/storage/cassandra/storage.py

swh/storage/in_memory.py

swh/storage/interface.py

swh/storage/retry.py

swh/storage/storage.py

swh/storage/tests/test_kafka_writer.py

swh/storage/tests/test_replay.py

swh/storage/tests/test_retry.py

swh/storage/tests/test_storage.py

swh/storage/validate.py

storage*: Align origin-visit-add to take iterable of OriginVisit objects as inputClosedPublicActions

Details

Diff Detail

Event Timeline

Patch application report for D3262 (id=11568)

Changes applied before test

Patch application report for D3262 (id=11569)

Changes applied before test

Patch application report for D3262 (id=11571)

Changes applied before test

Patch application report for D3262 (id=11572)

Changes applied before test

Patch application report for D3262 (id=11576)

Changes applied before test

Patch application report for D3262 (id=11578)

Changes applied before test

Patch application report for D3262 (id=11578)

Changes applied before test

Revision ContentsChangeset List

Diff 11576

requirements.txt

swh/storage/cassandra/storage.py

swh/storage/in_memory.py

swh/storage/interface.py

swh/storage/retry.py

swh/storage/storage.py

swh/storage/tests/test_kafka_writer.py

swh/storage/tests/test_replay.py

swh/storage/tests/test_retry.py

swh/storage/tests/test_storage.py

swh/storage/validate.py

storage*: Align origin-visit-add to take iterable of OriginVisit objects as input
ClosedPublic
Actions

Revision Contents
Changeset List