Page MenuHomeSoftware Heritage
Differential D3273

replay: Replay origin-visit and origin-visit-status
ClosedPublic
Actions

Authored by ardumont on Jun 12 2020, 12:02 PM.

Details

Reviewers
vlorentz
Group Reviewers
Reviewers
Maniphest Tasks
T2310: Make origin visits immutable
Commits
rDSTO2bcbc82b7085: replay: Replay origin-visit and origin-visit-status
Summary

This now uses the respective origin-visit-add and origin-visit-status-add
endpoints.

I'll clean up storage from origin-visit-upsert [2] (and origin-visit-update [1]) in
later diffs.

Related to T2310

[1] D3276

[2] D3281

Test Plan

tox

Diff Detail

Repository
rDSTO Storage manager
Branch
origin-visit-upsert
Lint
Lint Skipped
Unit
Unit Tests Skipped
Build Status
Buildable 12805
Build 19486: Phabricator diff pipeline on jenkinsJenkins console · Jenkins
Build 19485: arc lint + arc unit

Event Timeline

ardumont created this revision.Jun 12 2020, 12:02 PM
ardumont mentioned this in T2310: Make origin visits immutable.Jun 12 2020, 12:04 PM
swh-public-ci added a comment.Jun 12 2020, 12:10 PM

Build is green

Patch application report for D3273 (id=11600)

Rebasing onto 37c45300bc...

Current branch diff-target is up to date.
Changes applied before test
commit e519f6b3094320d1bb3aedbfc96af4819fae7652
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Thu Jun 11 17:16:29 2020 +0200

    replay: Migrate to use origin visit add instead of upsert
    
    Related to T2310

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

Harbormaster completed remote builds in B12805: Diff 11600.Jun 12 2020, 12:10 PM
vlorentz added a comment.Jun 12 2020, 12:53 PM

So how does the replayer create origin-visit-statuses?

ardumont added a comment.Jun 12 2020, 1:08 PM

So how does the replayer create origin-visit-statuses?

heh, thanks, i realize i'm missing an elif here!

Note that origin-visit-add currently adds the origin-visit-status for creation though.

Nonetheless, we should consume the object_type origin-visit-status indeed.

ardumont updated this revision to Diff 11608.Jun 12 2020, 1:33 PM

Fix missing origin-visit-status dealings

ardumont updated this revision to Diff 11609.Jun 12 2020, 1:34 PM

Use the wrong --head flag interval as i switched branch ¯\_(ツ)_/¯

ardumont updated this revision to Diff 11610.Jun 12 2020, 1:37 PM
  • Rework commit message
  • Remove added origin-visit-status object-type (forgot to remove it)
ardumont retitled this revision from replay: Migrate to use origin visit add instead of upsert to replay: Replay origin-visit and origin-visit-status.Jun 12 2020, 1:38 PM
ardumont edited the summary of this revision. (Show Details)
ardumont added inline comments.
swh/storage/replay.py
125

I used the dict because otherwise mypy would be a tad annoying me ;)

swh-public-ci added a comment.Jun 12 2020, 1:41 PM

Build is green

Patch application report for D3273 (id=11608)

Rebasing onto 37c45300bc...

First, rewinding head to replay your work on top of it...
Fast-forwarded diff-target to base-revision-262-D3273.
Changes applied before test



See https://jenkins.softwareheritage.org/job/DSTO/job/tests-on-diff/262/ for more details.
Harbormaster completed remote builds in B12813: Diff 11608.Jun 12 2020, 1:41 PM
swh-public-ci added a comment.Jun 12 2020, 1:42 PM
Build was aborted



Patch application report for D3273 (id=11609)



Rebasing onto 37c45300bc...



Current branch diff-target is up to date.





Changes applied before test



commit 4541ae40ee940ac5f193aa372020ec74ff80034b
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Thu Jun 11 17:16:29 2020 +0200

    replay: Migrate to use origin visit add instead of upsert
    
    Related to T2310



Link to build: https://jenkins.softwareheritage.org/job/DSTO/job/tests-on-diff/263/

See console output for more information: https://jenkins.softwareheritage.org/job/DSTO/job/tests-on-diff/263/console
Harbormaster failed remote builds in B12814: Diff 11609!Jun 12 2020, 1:42 PM
swh-public-ci added a comment.Jun 12 2020, 1:50 PM
Build is green



Patch application report for D3273 (id=11610)



Rebasing onto 37c45300bc...



Current branch diff-target is up to date.





Changes applied before test



commit 5e096bb4cf1b0236eb6d2cd30848a35a99b793ad
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Thu Jun 11 17:16:29 2020 +0200

    replay: Replay origin-visit and origin-visit-status
    
    This now uses the respective origin-visit-add and origin-visit-status-add
    endpoints.
    
    Related to T2310



See https://jenkins.softwareheritage.org/job/DSTO/job/tests-on-diff/264/ for more details.
Harbormaster completed remote builds in B12815: Diff 11610.Jun 12 2020, 1:50 PM
vlorentz added a comment.Jun 12 2020, 2:41 PM
could you add a test for statuses?
vlorentz added a comment.Jun 12 2020, 2:42 PM
s/obj_models/model_objs/?
ardumont added a comment.Jun 12 2020, 2:54 PM
could you add a test for statuses?



my current understanding is that it's tested already but sure, might as well make it explicit.



s/obj_models/model_objs/?



sure ;)
vlorentz added a comment.Jun 12 2020, 3:14 PM
my current understanding is that it's tested already but sure, might as well make it explicit.



If it was, you would have caught the missing handling of statuses
ardumont added a comment.EditedJun 12 2020, 3:21 PM
my current understanding is that it's tested already but sure, might as well make it explicit.



If it was, you would have caught the missing handling of statuses



ugh, right! That's... not false! ;D

It's writing visit-statuses but it's not asserting anything about them at the end of tests.



Currently fighting the new assertions which for now demonstrates more many origin-visit-statuses are written than expected.

Semi-expectedly since origin-visit-add writes visit-status as well...



I think the rest is in-memory implementation specific which is annoying.

(~> I've no idea yet if i messed up the in-memory implementation or not)
ardumont updated this revision to Diff 11614.Jun 12 2020, 3:23 PM
ardumont edited the summary of this revision. (Show Details)
    

  • Adapt according to review
    • 

  • Start checking the visit-status writes (which fails for now, the error will be shared that way ;)
    • 

swh-public-ci added a comment.Jun 12 2020, 3:31 PM
Build has FAILED



Patch application report for D3273 (id=11614)



Rebasing onto 37c45300bc...



Current branch diff-target is up to date.





Changes applied before test



commit a9ea6235af9cdf8a08f4650f97ac20feac9f8480
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Thu Jun 11 17:16:29 2020 +0200

    replay: Replay origin-visit and origin-visit-status
    
    This now uses the respective origin-visit-add and origin-visit-status-add
    endpoints.
    
    Related to T2310



Link to build: https://jenkins.softwareheritage.org/job/DSTO/job/tests-on-diff/265/

See console output for more information: https://jenkins.softwareheritage.org/job/DSTO/job/tests-on-diff/265/console
Harbormaster failed remote builds in B12820: Diff 11614!Jun 12 2020, 3:31 PM
ardumont planned changes to this revision.Jun 12 2020, 4:01 PM
ardumont mentioned this in D3278: in_memory: Make origin-visit-status-add respect "on conflict ignore" policy.Jun 13 2020, 8:42 AM
ardumont updated this revision to Diff 11621.Jun 13 2020, 8:49 AM
    

  • Rebase on top of diff that should make the build ok
    • 

  • test_storage_play_anonymized: Skip origin-visit-status as well (aligning with origin-visit)
    • 

  • Drop the origin-visit-update part which is no longer relevant there
    • 




Depends on D3278
ardumont added inline comments.Jun 13 2020, 8:51 AM
swh/storage/tests/test_kafka_writer.py


63 ↗(On Diff #11621)
maybe i should drop that when we remove origin-visit-update [1]?



[1] D3276
ardumont updated this revision to Diff 11622.Jun 13 2020, 9:00 AM
Remove the part touching the origin-visit-update call, i'll do that in the

dedicated diff.
swh-public-ci added a comment.Jun 13 2020, 9:03 AM
Build is green



Patch application report for D3273 (id=11621)



Could not rebase; Attempt merge onto 33efdb0dd4...



Updating 33efdb0..0d970bc
Fast-forward
 swh/storage/in_memory.py               |  6 +++--
 swh/storage/replay.py                  | 15 ++++++-----
 swh/storage/tests/test_kafka_writer.py |  8 +-----
 swh/storage/tests/test_replay.py       |  7 +++---
 swh/storage/tests/test_storage.py      | 46 +++++++++++++++++++++++++---------
 swh/storage/validate.py                | 32 +++++++++++++++--------
 6 files changed, 74 insertions(+), 40 deletions(-)





Changes applied before test



commit 0d970bcde0ab3b24d4fe03fe05940923fac74888
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Thu Jun 11 17:16:29 2020 +0200

    replay: Replay origin-visit and origin-visit-status
    
    This now uses the respective origin-visit-add and origin-visit-status-add
    endpoints.
    
    Related to T2310

commit 1d8a54ba8312d15c9fd843efe3754691576398af
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Sat Jun 13 08:40:29 2020 +0200

    in_memory: Make origin-visit-status-add do on conflict ignore
    
    Aligning its behavior with the other backend.
    
    Related to T2310

commit 874da2dee1957ac5e9f5b17a81e11d9d9e8690f0
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Sat Jun 13 08:37:57 2020 +0200

    Start migrating the validate proxy toward using BaseModel objects
    
    This will allow to progress incrementally towards removing it.
    
    When it allows to use BaseModel objects everywhere (and tests in test_storage
    are adapted to use this property), it will be time to remove it entirely (as
    it's only used in test).
    
    It's preparatory work for future diffs.



See https://jenkins.softwareheritage.org/job/DSTO/job/tests-on-diff/270/ for more details.
Harbormaster completed remote builds in B12828: Diff 11621.Jun 13 2020, 9:03 AM
ardumont mentioned this in D3276: storage*: Remove origin-visit-update endpoint.Jun 13 2020, 9:05 AM
ardumont added a comment.Jun 13 2020, 9:08 AM
I think the rest is in-memory implementation specific which is annoying.

(~> I've no idea yet if i messed up the in-memory implementation or not)



in-memory implementation was not respecting the on-conflict ignore insertion

the other backends do. So here goes D3278 to fix that.
swh-public-ci added a comment.Jun 13 2020, 9:10 AM
Build has FAILED



Patch application report for D3273 (id=11622)



Could not rebase; Attempt merge onto 33efdb0dd4...



Updating 33efdb0..645f104
Fast-forward
 swh/storage/in_memory.py          |  6 +++--
 swh/storage/replay.py             | 15 ++++++++-----
 swh/storage/tests/test_replay.py  |  7 +++---
 swh/storage/tests/test_storage.py | 46 +++++++++++++++++++++++++++++----------
 swh/storage/validate.py           | 32 ++++++++++++++++++---------
 5 files changed, 73 insertions(+), 33 deletions(-)





Changes applied before test



commit 645f104bd401dc0399bbdc61349210ad81300f88
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Thu Jun 11 17:16:29 2020 +0200

    replay: Replay origin-visit and origin-visit-status
    
    This now uses the respective origin-visit-add and origin-visit-status-add
    endpoints.
    
    Related to T2310

commit 1d8a54ba8312d15c9fd843efe3754691576398af
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Sat Jun 13 08:40:29 2020 +0200

    in_memory: Make origin-visit-status-add do on conflict ignore
    
    Aligning its behavior with the other backend.
    
    Related to T2310

commit 874da2dee1957ac5e9f5b17a81e11d9d9e8690f0
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Sat Jun 13 08:37:57 2020 +0200

    Start migrating the validate proxy toward using BaseModel objects
    
    This will allow to progress incrementally towards removing it.
    
    When it allows to use BaseModel objects everywhere (and tests in test_storage
    are adapted to use this property), it will be time to remove it entirely (as
    it's only used in test).
    
    It's preparatory work for future diffs.



Link to build: https://jenkins.softwareheritage.org/job/DSTO/job/tests-on-diff/271/

See console output for more information: https://jenkins.softwareheritage.org/job/DSTO/job/tests-on-diff/271/console
Harbormaster failed remote builds in B12829: Diff 11622!Jun 13 2020, 9:10 AM
swh-public-ci added a comment.Jun 13 2020, 9:25 AM
Build is green



Patch application report for D3273 (id=11622)



Could not rebase; Attempt merge onto 33efdb0dd4...



Updating 33efdb0..645f104
Fast-forward
 swh/storage/in_memory.py          |  6 +++--
 swh/storage/replay.py             | 15 ++++++++-----
 swh/storage/tests/test_replay.py  |  7 +++---
 swh/storage/tests/test_storage.py | 46 +++++++++++++++++++++++++++++----------
 swh/storage/validate.py           | 32 ++++++++++++++++++---------
 5 files changed, 73 insertions(+), 33 deletions(-)





Changes applied before test



commit 645f104bd401dc0399bbdc61349210ad81300f88
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Thu Jun 11 17:16:29 2020 +0200

    replay: Replay origin-visit and origin-visit-status
    
    This now uses the respective origin-visit-add and origin-visit-status-add
    endpoints.
    
    Related to T2310

commit 1d8a54ba8312d15c9fd843efe3754691576398af
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Sat Jun 13 08:40:29 2020 +0200

    in_memory: Make origin-visit-status-add do on conflict ignore
    
    Aligning its behavior with the other backend.
    
    Related to T2310

commit 874da2dee1957ac5e9f5b17a81e11d9d9e8690f0
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Sat Jun 13 08:37:57 2020 +0200

    Start migrating the validate proxy toward using BaseModel objects
    
    This will allow to progress incrementally towards removing it.
    
    When it allows to use BaseModel objects everywhere (and tests in test_storage
    are adapted to use this property), it will be time to remove it entirely (as
    it's only used in test).
    
    It's preparatory work for future diffs.



See https://jenkins.softwareheritage.org/job/DSTO/job/tests-on-diff/273/ for more details.
Harbormaster completed remote builds in B12829: Diff 11622.Jun 13 2020, 9:25 AM
vlorentz accepted this revision.Jun 15 2020, 11:02 AM
This revision is now accepted and ready to land.Jun 15 2020, 11:02 AM
ardumont updated this revision to Diff 11637.Jun 15 2020, 2:09 PM
Rebase on latest master
ardumont edited the summary of this revision. (Show Details)Jun 15 2020, 2:10 PM
swh-public-ci added a comment.Jun 15 2020, 2:17 PM
Build is green



Patch application report for D3273 (id=11637)



Rebasing onto 0183fec91d...



Current branch diff-target is up to date.





Changes applied before test



commit 2bcbc82b708550415ce23a8a91830ad2b75f848c
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Thu Jun 11 17:16:29 2020 +0200

    replay: Replay origin-visit and origin-visit-status
    
    This now uses the respective origin-visit-add and origin-visit-status-add
    endpoints.
    
    Related to T2310



See https://jenkins.softwareheritage.org/job/DSTO/job/tests-on-diff/279/ for more details.
Harbormaster completed remote builds in B12844: Diff 11637.Jun 15 2020, 2:17 PM
ardumont mentioned this in D3281: storage*: Drop origin_visit_upsert endpoint.Jun 15 2020, 2:27 PM
Closed by commit rDSTO2bcbc82b7085: replay: Replay origin-visit and origin-visit-status (authored by ardumont).  ·  Explain WhyJun 15 2020, 2:28 PM
This revision was automatically updated to reflect the committed changes.
ardumont added a commit: rDSTO2bcbc82b7085: replay: Replay origin-visit and origin-visit-status.
ardumont edited the summary of this revision. (Show Details)Jun 15 2020, 2:30 PM
Revision Contents
Changeset List
PathSizeCoverage (All)Coverage (Touched)
swh/
storage/
2 lines98%
Loading...
Diff 11600
View Options
swh/storage/replay.py
Loading...
About · Developer information · Legal