Page MenuHomeSoftware Heritage
Differential D3344

journal_data: Make origin-visit optional fields to None
ClosedPublic
Actions

Authored by ardumont on Jun 24 2020, 1:40 PM.

Details

Reviewers
olasd
Group Reviewers
Reviewers
Maniphest Tasks
T2310: Make origin visits immutable
Commits
rDJNL4c7d945492a7: journal_data: Make origin-visit optional fields to None
Summary

This sets the fields status, metadata and snapshot from the origin-visit
entries to None (as per the recent model change D3340)

This pulls the creation of origin-visit-status with almost the same
origin-visit entries as before. the origin-visit-status dates are slightly larger than the
origin-visit ones so we are able to reflect the reality of operations (first
creation of origin visits, then creation of origin-visit-status associated to
that origin-visit).

Related to D3342

Related to T2310

Test Plan

tox

Diff Detail

Repository
rDJNL Journal infrastructure
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

ardumont created this revision.Jun 24 2020, 1:40 PM
Herald added a reviewer: Reviewers. · View Herald TranscriptJun 24 2020, 1:40 PM
ardumont added a child revision: D3345: journal: Drop datetime conversion indirection.Jun 24 2020, 1:41 PM
swh-public-ci added a comment.Jun 24 2020, 1:42 PM

Build is green

Patch application report for D3344 (id=11857)

Rebasing onto 6ea2d6e4c8...

Current branch diff-target is up to date.
Changes applied before test
commit 7d3d0062c9b63d72ff1033fdbad5b4a0b009209f
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Wed Jun 24 13:38:30 2020 +0200

    journal_data: Make origin-visit optional fields to None
    
    Related to T2310

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

Harbormaster completed remote builds in B13072: Diff 11857.Jun 24 2020, 1:42 PM
ardumont added inline comments.Jun 24 2020, 2:21 PM
swh/journal/tests/journal_data.py
185

Those dates needs to be shifted slightly in the future from their visit counterpart.
Otherwise, we are hitting storage-wise the "on conflict" ignore policy (because origin-visit-add creates an origin-visit-status with the same parameters from the origin-visit {origin, visit, date}...

ardumont updated this revision to Diff 11859.Jun 24 2020, 2:25 PM

Shift slightly origin-visit-status date to the future.

So we won't hit the on-conflit ignore which does not make sense in the real
life. real life: We will always create origin-visit-status after the initial
origin-visit. And that operation won't happen at the same millisecond.

swh-public-ci added a comment.Jun 24 2020, 2:27 PM

Build is green

Patch application report for D3344 (id=11859)

Rebasing onto 6ea2d6e4c8...

Current branch diff-target is up to date.
Changes applied before test
commit 12f3f2082e53240a4c050e94e008e71e25dcdded
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Wed Jun 24 13:38:30 2020 +0200

    journal_data: Make origin-visit optional fields to None
    
    Related to T2310

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

Harbormaster completed remote builds in B13074: Diff 11859.Jun 24 2020, 2:27 PM
ardumont edited the summary of this revision. (Show Details)Jun 24 2020, 2:27 PM
ardumont edited the summary of this revision. (Show Details)Jun 24 2020, 2:31 PM
ardumont mentioned this in D3342: storage*: Drop obsolete fields from origin_visit.Jun 24 2020, 2:36 PM
olasd requested changes to this revision.Jun 24 2020, 3:54 PM
olasd added a subscriber: olasd.
olasd added inline comments.
swh/journal/tests/journal_data.py
185

This needs to be a comment in the code, and I'd even go as far as saying there should be a test for this so we don't mess it up in the future.

This revision now requires changes to proceed.Jun 24 2020, 3:54 PM
olasd added a comment.Jun 24 2020, 3:56 PM

(the suggested test should check that, for each ORIGIN_VISIT_STATUS, the date is > to the one of the matching ORIGIN_VISIT)

ardumont added inline comments.Jun 24 2020, 4:15 PM
swh/journal/tests/journal_data.py
185

yes, right...

I spent so much time updating (comment and description), that then after that, i fool myself into thinking, there it's ok.

Thanks.

ardumont updated this revision to Diff 11869.Jun 24 2020, 4:28 PM
ardumont edited the summary of this revision. (Show Details)

Adapt according to review

swh-public-ci added a comment.Jun 24 2020, 4:30 PM

Build is green

Patch application report for D3344 (id=11869)

Rebasing onto 6ea2d6e4c8...

Current branch diff-target is up to date.
Changes applied before test
commit a5143c05b91451df4d17c7e919db8d807ae073ac
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Wed Jun 24 13:38:30 2020 +0200

    journal_data: Make origin-visit optional fields to None
    
    This sets the fields status, metadata and snapshot from the origin-visit
    entries to None (as per the recent model change D3340)
    
    This pulls the creation of origin-visit-status with almost the same
    origin-visit entries as before. The origin-visit-status dates are slightly
    larger than the origin-visit ones so we are able to reflect the reality of
    operations (first creation of origin visits, then creation of
    origin-visit-status associated to that origin-visit).
    
    Related to T2310

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

Harbormaster completed remote builds in B13083: Diff 11869.Jun 24 2020, 4:30 PM
ardumont edited the summary of this revision. (Show Details)Jun 24 2020, 4:31 PM
olasd accepted this revision.Jun 24 2020, 6:09 PM

Maybe the asserts should be in an actual test function? Feels a bit wrong to have top-level asserts in that file.

swh/journal/tests/journal_data.py
235–239

could use zip instead of enumerate here.

This revision is now accepted and ready to land.Jun 24 2020, 6:09 PM
ardumont updated this revision to Diff 11875.Jun 25 2020, 9:25 AM

Adapt according to review:

  • Move assertions on origin-visit-status dates to a dedicated test
  • Use zip instead of enumerate
swh-public-ci added a comment.Jun 25 2020, 9:27 AM

Build is green

Patch application report for D3344 (id=11875)

Rebasing onto 6ea2d6e4c8...

Current branch diff-target is up to date.
Changes applied before test
commit 4c7d945492a7e021b1c3ddd5cb17f7194cacad9e
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Wed Jun 24 13:38:30 2020 +0200

    journal_data: Make origin-visit optional fields to None
    
    This sets the fields status, metadata and snapshot from the origin-visit
    entries to None (as per the recent model change D3340)
    
    This pulls the creation of origin-visit-status with almost the same
    origin-visit entries as before. The origin-visit-status dates are slightly
    larger than the origin-visit ones so we are able to reflect the reality of
    operations (first creation of origin visits, then creation of
    origin-visit-status associated to that origin-visit).
    
    Related to T2310

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

Harbormaster completed remote builds in B13089: Diff 11875.Jun 25 2020, 9:27 AM
Closed by commit rDJNL4c7d945492a7: journal_data: Make origin-visit optional fields to None (authored by ardumont). · Explain WhyJun 25 2020, 9:29 AM
This revision was automatically updated to reflect the committed changes.
ardumont added a commit: rDJNL4c7d945492a7: journal_data: Make origin-visit optional fields to None.
ardumont mentioned this in T2310: Make origin visits immutable.Jun 25 2020, 7:21 PM

Revision Contents
Changeset List

PathSize
swh/
journal/
tests/
80 lines
23 lines

Diff 11876

View Options

swh/journal/tests/journal_data.py

Loading...
View Options

swh/journal/tests/test_journal_data.py

Loading...
About · Developer information · Legal