Page MenuHomeSoftware Heritage
Differential D2813

origin_visit_upsert: Use OriginVisit objects as input
ClosedPublic
Actions

Authored by ardumont on Mar 11 2020, 5:38 PM.

Details

Reviewers
vlorentz
Group Reviewers
Reviewers
Commits
rDSTO0456cce5a0d4: origin_visit_upsert: Use OriginVisit object as input
Summary

This aligns with other _add endpoints [1]

Impacts:

  • D2826: swh-journal (replayer)

Related to D2812#67298

[1] except for origin_visit_add which is next (D2820)

Test Plan

tox

Diff Detail

Repository
rDSTO Storage manager
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.Mar 11 2020, 5:38 PM
Herald added a reviewer: Reviewers. · View Herald TranscriptMar 11 2020, 5:38 PM
ardumont mentioned this in D2812: journal: Use swh-model objects instead of dicts in replay and writer.Mar 11 2020, 5:41 PM
swh-public-ci added a comment.Mar 11 2020, 5:44 PM

Build is green
See https://jenkins.softwareheritage.org/job/DSTO/job/tox/1044/ for more details.

Harbormaster completed remote builds in B11076: Diff 10004.Mar 11 2020, 5:44 PM
ardumont added a child revision: D2820: storage: origin_visit_add: Type and make origin_visit_add return an OriginVisit object.Mar 12 2020, 2:21 AM
ardumont edited the summary of this revision. (Show Details)Mar 12 2020, 2:22 AM
ardumont edited the summary of this revision. (Show Details)
ardumont edited the summary of this revision. (Show Details)Mar 12 2020, 10:45 AM
vlorentz requested changes to this revision.Mar 12 2020, 2:27 PM
vlorentz added a subscriber: vlorentz.
vlorentz added inline comments.
swh/storage/cassandra/storage.py
829

This shouldn't type-check.

Call json.dumps in CqlRunner.origin_visit_upsert, like CqlRunner.origin_visit_update does.

swh/storage/db.py
382

Do we want swh-model objects in db.py?

This revision now requires changes to proceed.Mar 12 2020, 2:27 PM
ardumont added inline comments.Mar 12 2020, 2:44 PM
swh/storage/db.py
382

why not?

vlorentz added inline comments.Mar 12 2020, 3:10 PM
swh/storage/db.py
382

Because we didn't do it so far, so this introduces an inconsistency.

ardumont added inline comments.Mar 12 2020, 6:23 PM
swh/storage/db.py
382

but we are trying to fix that inconsistency everywhere by using model object...
I'm trying to get there...

ardumont added inline comments.Mar 12 2020, 6:24 PM
swh/storage/cassandra/storage.py
829

ok, will do.

ardumont edited the summary of this revision. (Show Details)Mar 12 2020, 6:24 PM
ardumont added inline comments.Mar 12 2020, 6:28 PM
swh/storage/db.py
382

also you typed cql with base model object
This cql (cassandra) module is the equivalent of the db (pg) here...
So that's the consistent way forward.

vlorentz added inline comments.Mar 12 2020, 6:38 PM
swh/storage/db.py
382

Sure, but I would rather this single function is consistent with the rest of its module, than with a completely different module.

But if we want to make db.py use model objects, then sure

ardumont added inline comments.Mar 12 2020, 6:41 PM
swh/storage/cassandra/storage.py
829

This shouldn't type-check.

metadata = attr.ib(type=Optional[Dict[str, object]],
                   default=None)

I see now. json.dumps could return a string, list or Dict...

Well, that's actually a field which is not used...

ardumont added inline comments.Mar 12 2020, 6:46 PM
swh/storage/cassandra/storage.py
829

json.dumps could return a string, list or Dict...

no, just a string...
ok i see (pfff, needs some nap)

ardumont updated this revision to Diff 10038.Mar 12 2020, 7:09 PM

Adapt according to review

swh-public-ci added a comment.Mar 12 2020, 7:47 PM

Build was aborted

Link to build: https://jenkins.softwareheritage.org/job/DSTO/job/tox/1055/
See console output for more information: https://jenkins.softwareheritage.org/job/DSTO/job/tox/1055/console

Harbormaster failed remote builds in B11109: Diff 10038!Mar 12 2020, 7:47 PM
ardumont edited the summary of this revision. (Show Details)Mar 12 2020, 8:21 PM
swh-public-ci added a comment.Mar 13 2020, 9:41 AM

Build was aborted

Link to build: https://jenkins.softwareheritage.org/job/DSTO/job/tox/1056/
See console output for more information: https://jenkins.softwareheritage.org/job/DSTO/job/tox/1056/console

swh-public-ci added a comment.Mar 13 2020, 9:59 AM

Build is green
See https://jenkins.softwareheritage.org/job/DSTO/job/tox/1059/ for more details.

Harbormaster completed remote builds in B11109: Diff 10038.Mar 13 2020, 9:59 AM
ardumont added inline comments.Mar 13 2020, 10:01 AM
swh/storage/db.py
382

Well consistency is the goal.
But i'm doing that incrementally as i go along.

vlorentz accepted this revision.Mar 13 2020, 11:31 AM

👍

This revision is now accepted and ready to land.Mar 13 2020, 11:31 AM
Closed by commit rDSTO0456cce5a0d4: origin_visit_upsert: Use OriginVisit object as input (authored by ardumont). · Explain WhyMar 13 2020, 12:02 PM
This revision was automatically updated to reflect the committed changes.
ardumont added a commit: rDSTO0456cce5a0d4: origin_visit_upsert: Use OriginVisit object as input.
ardumont removed a child revision: D2820: storage: origin_visit_add: Type and make origin_visit_add return an OriginVisit object.Mar 13 2020, 2:05 PM
ardumont mentioned this in D2826: journal: Migrate to latest storage.origin_visit_upsert/add api change.Mar 13 2020, 3:47 PM
ardumont edited the summary of this revision. (Show Details)Mar 14 2020, 11:42 AM
ardumont mentioned this in rDJNLd3d5b7977186: Migrate to latest origin_visit_upsert/add api changes.Mar 16 2020, 3:20 PM

Revision Contents
Changeset List

PathSize
swh/
storage/
cassandra/
13 lines
11 lines
11 lines
18 lines
6 lines
19 lines
tests/
56 lines

Diff 10043

View Options

swh/storage/cassandra/cql.py

Loading...
View Options

swh/storage/cassandra/storage.py

Loading...
View Options

swh/storage/db.py

Loading...
View Options

swh/storage/in_memory.py

Loading...
View Options

swh/storage/interface.py

Loading...
View Options

swh/storage/storage.py

Loading...
View Options

swh/storage/tests/test_storage.py

Loading...
About · Developer information · Legal