Page MenuHomeSoftware Heritage

Generalize origin_metadata to allow support for other object types in the future.
ClosedPublic

Authored by vlorentz on Thu, Jun 25, 5:47 PM.

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

vlorentz created this revision.Thu, Jun 25, 5:47 PM

Build is green

Patch application report for D3355 (id=11904)

Rebasing onto b991e69707...

First, rewinding head to replay your work on top of it...
Applying: Generalize origin_metadata to allow support for other object types in the future.
Changes applied before test
commit 702cf25134d32b9e5010abc728401f02af01c08f
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Thu Jun 25 17:42:31 2020 +0200

    Generalize origin_metadata to allow support for other object types in the future.

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

Nice!

Looks fine to me ;)

Got a couple of nits/remarks above.
Overall, i'm only missing edge case scenario (based on the coverage report).

sql/upgrades/156.sql
42 ↗(On Diff #11904)

storage deployment is gonna be fun... (replication wise ;)

swh/storage/cassandra/cql.py
888

i just realized we could do that heh ;)

(build stuff within the decorator)

967

types?

swh/storage/in_memory.py
1017

Remove the extra pass ;)

swh/storage/storage.py
1167

could you add a test for that part?

1227

same here.

ardumont accepted this revision.Fri, Jun 26, 4:07 PM

Please, don't merge it before D3350 so i can deploy in 2 unrelated steps ;)

This revision is now accepted and ready to land.Fri, Jun 26, 4:07 PM
vlorentz added inline comments.Fri, Jun 26, 5:51 PM
swh/storage/in_memory.py
1017

I'm blaming git

ardumont added inline comments.Fri, Jun 26, 5:52 PM
swh/storage/in_memory.py
1017

:D

I'll remove the origin checks instead, they are not needed anymore.

vlorentz updated this revision to Diff 11948.Mon, Jun 29, 11:42 AM

apply comments

Build has FAILED

Patch application report for D3355 (id=11948)

Rebasing onto 10443b8a17...

First, rewinding head to replay your work on top of it...
Applying: Generalize origin_metadata to allow support for other object types in the future.
Changes applied before test
commit d8fa4b9d5e75f4d12aa8338a5bc5b35450fe8bb1
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Thu Jun 25 17:42:31 2020 +0200

    Generalize origin_metadata to allow support for other object types in the future.

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

Build is green

Patch application report for D3355 (id=11948)

Rebasing onto 10443b8a17...

First, rewinding head to replay your work on top of it...
Applying: Generalize origin_metadata to allow support for other object types in the future.
Changes applied before test
commit 9d064fb0a018abbc7a774067377b46acfd81d251
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Thu Jun 25 17:42:31 2020 +0200

    Generalize origin_metadata to allow support for other object types in the future.

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

douardda added inline comments.
swh/storage/sql/30-swh-schema.sql
443

shouldn't there also be a description for the type column?

What are the expected values for this? I guess it's an enum-kind of field right?

vlorentz updated this revision to Diff 11963.Mon, Jun 29, 6:01 PM

rebase + apply comment

vlorentz marked an inline comment as done.Mon, Jun 29, 6:01 PM
vlorentz added inline comments.
swh/storage/sql/30-swh-schema.sql
443

I added the list of values. Yes, it's like an enum, but I didn't use one because it doesn't match any of the enums we currently have (all DAG objects plus origins, but not visits)

Build has FAILED

Patch application report for D3355 (id=11963)

Rebasing onto 10443b8a17...

Current branch diff-target is up to date.
Changes applied before test
commit 595d5462e14f010b6acae3e87ff6aa7ce75ae8ce
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Thu Jun 25 17:42:31 2020 +0200

    Generalize origin_metadata to allow support for other object types in the future.

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

Build is green

Patch application report for D3355 (id=11980)

Rebasing onto 1f0e25615e...

Current branch diff-target is up to date.
Changes applied before test
commit 27e942621cef91cea6a98ffcc7beed5935714fe8
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Thu Jun 25 17:42:31 2020 +0200

    Generalize origin_metadata to allow support for other object types in the future.

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