Differential D3238

origin-visit-add: Write origin-visit-status objects to the journal
ClosedPublic
Actions

Authored by ardumont on Jun 8 2020, 11:56 AM.

Details

Reviewers

vlorentz

Group Reviewers

Reviewers

Maniphest Tasks

T2310: Make origin visits immutable

Commits

rDSTO0860920774d1: origin-visit-add: Write visit status to the journal

Summary

This also makes the instruction order consistent across the different storage
implementations. First, write objects to the journal, then write objects to the
storage backend.

This does not deal yet with endpoints origin-visit-update and origin-visit-upsert.

Related to T2310

Test Plan

tox

Diff Detail

Repository

rDSTO Storage manager

Branch

master

Lint

No Linters Available

Unit

No Unit Test Coverage

Build Status

Buildable 12690
Build 19291: Phabricator diff pipeline on jenkins	Jenkins console · Jenkins
Build 19290: arc lint + arc unit

Event Timeline

ardumont created this revision.Jun 8 2020, 11:56 AM

Build was aborted

Patch application report for D3238 (id=11485)

Rebasing onto dcef916e5e...

Current branch diff-target is up to date.

Changes applied before test

commit 18de8f6087d230dcd7d09e11c1e4a99af9cb0066
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Mon Jun 8 11:42:13 2020 +0200

    origin-visit-{add|update}: Write origin-visit-status-add to the journal
    
    This also makes the instruction order consistent across the different storage
    implementations. First, write objects to the journal, then write objects to the
    storage backend.
    
    Related to T2310

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

Harbormaster failed remote builds in B12690: Diff 11485!Jun 8 2020, 12:23 PM

ardumont planned changes to this revision.Jun 8 2020, 2:00 PM

ardumont retitled this revision from origin-visit-{add|update}: Write origin-visit-status-add to the journal to origin-visit-add: Write origin-visit-status to the journal.

ardumont edited the summary of this revision. (Show Details)

I need to update the journal module for origin-visit-status as dependencies.

ardumont mentioned this in D3241: Allow journal to deal with origin_visit_status.Jun 8 2020, 2:28 PM

Reduce scope to only origin-visit-add (which now also write origin-visit-status
to journal)

Other diffs will come to deal with origin-visit-update and origin-visit-upsert.

ardumont edited the test plan for this revision. (Show Details)Jun 8 2020, 2:37 PM

ardumont edited the test plan for this revision. (Show Details)

ardumont added a project: Storage manager.

ardumont edited the test plan for this revision. (Show Details)

ardumont added inline comments.Jun 8 2020, 2:39 PM

swh/storage/in_memory.py
903	I just realigned the implementation with the other backends. This also aligns the journal order writes (which is tested and failed due to origin-visit-status being written there now)

Build has FAILED

Patch application report for D3238 (id=11489)

Rebasing onto dcef916e5e...

Current branch diff-target is up to date.

Changes applied before test

commit 3c0203cac19396d70f6770f6962d598c31ce69c0
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Mon Jun 8 11:42:13 2020 +0200

    origin-visit-add: Write visit status to the journal
    
    This also makes the instruction order consistent across the different storage
    implementations. First, write objects to the journal, then write objects to the
    storage backend.
    
    Related to T2310

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

Harbormaster failed remote builds in B12694: Diff 11489!Jun 8 2020, 2:41 PM

vlorentz added inline comments.Jun 8 2020, 2:46 PM

swh/storage/cassandra/storage.py
899–900	why this change?

ardumont added inline comments.Jun 8 2020, 2:50 PM

swh/storage/cassandra/storage.py
899–900	It's only a temporary change. Now that self._origin_visit_status_add writes to the journal, the journal testing part is a mess of origin-visit and origin-visit status with differing date... I want to concern myself here only with origin-visit-add without the harassment i had so far with the update part. So i'll deal with that in another diff which will revert that behavior (thus why it's commented). If you want I can add that as a FIXME.

vlorentz accepted this revision.Jun 8 2020, 2:52 PM

This revision is now accepted and ready to land.Jun 8 2020, 2:52 PM

ardumont added a child revision: D3244: origin-visit-update: Write visit-status objects to the journal.Jun 8 2020, 4:28 PM

ardumont added inline comments.Jun 8 2020, 5:12 PM

swh/storage/cassandra/storage.py
899–900	So i'll deal with that in another diff which will revert that behavior (thus why it's commented). D3244 ;)

Build has FAILED

Patch application report for D3238 (id=11489)

Rebasing onto 7eb44d412b...

First, rewinding head to replay your work on top of it...
Applying: origin-visit-add: Write visit status to the journal

Changes applied before test

commit d2308540cc934c30b0459f50eed017791b896c25
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Mon Jun 8 11:42:13 2020 +0200

    origin-visit-add: Write visit status to the journal
    
    This also makes the instruction order consistent across the different storage
    implementations. First, write objects to the journal, then write objects to the
    storage backend.
    
    Related to T2310

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

ardumont edited the test plan for this revision. (Show Details)Jun 9 2020, 12:22 PM

ardumont mentioned this in D3249: pytest_plugin: Fix cascading assertion error when key is not hashable.Jun 9 2020, 12:51 PM

Rebase on latest master
Fix journal related tests to take into accounts origin-visit-status objects

ardumont retitled this revision from origin-visit-add: Write origin-visit-status to the journal to origin-visit-add: Write origin-visit-status objects to the journal.Jun 9 2020, 2:35 PM

Build is green

Patch application report for D3238 (id=11517)

Rebasing onto 7eb44d412b...

Current branch diff-target is up to date.

Changes applied before test

commit 0860920774d1f907047443b4f0604ee6cbe2889b
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Mon Jun 8 11:42:13 2020 +0200

    origin-visit-add: Write visit status to the journal
    
    This also makes the instruction order consistent across the different storage
    implementations. First, write objects to the journal, then write objects to the
    storage backend.
    
    Related to T2310

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

Harbormaster completed remote builds in B12721: Diff 11517.Jun 9 2020, 2:38 PM

ardumont edited the test plan for this revision. (Show Details)Jun 9 2020, 2:38 PM

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

Closed by commit rDSTO0860920774d1: origin-visit-add: Write visit status to the journal (authored by ardumont). · Explain WhyJun 10 2020, 9:06 AM

This revision was automatically updated to reflect the committed changes.

ardumont added a commit: rDSTO0860920774d1: origin-visit-add: Write visit status to the journal.