Page MenuHomeSoftware Heritage

journal_data: Make origin-visit optional fields to None
ClosedPublic

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

Details

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

Event Timeline

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.

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}...

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.

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.

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

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

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 edited the summary of this revision. (Show Details)

Adapt according to review

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.

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
236–240

could use zip instead of enumerate here.

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

Adapt according to review:

  • Move assertions on origin-visit-status dates to a dedicated test
  • Use zip instead of enumerate

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.