Page MenuHomeSoftware Heritage
Differential D5072

Refactor the db scaffolding to use swh.core.db
ClosedPublic
Actions

Authored by douardda on Feb 12 2021, 4:31 PM.

Details

Reviewers
aeviso
Group Reviewers
Reviewers
Commits
rDPROV5d3ef241e86e: Refactor the db initialization scaffolding to use swh.core.db
rDPROVdabbd5334967: Refactor the database creation code and a test for the cli command
Summary

There are several parts in this diff (should be in reasnably correctly splittes git revsions):

  • split the 2 provenance.sql scripts in parts and move them to swh/provenance/sql so the database creation and initialization are handled by the swh db init and swh db create tools from swh.core.db.
  • with/witout path choice is implemented using 2 flavors: with-path and without-path. So the initialization command should be something like:

    swh db init -d db-name --flavor with-path provenance
  • refactor the database creation code and a test for the cli command
  • this later evolution comes with a few changes in the configuration file format to "normalize" it a bit and ease testing:
    • 'ps' config cls for the archive has been renamed as 'direct'
    • 'ps' confif cls for the provenance db has been renamed as 'local' (similar to other swh packages),
    • 'ps_np' has been suppressed in favor of a 'with_path' config flag in the 'provenance' section.
  • also add a provenance fixture in a conftest.py file.

Diff Detail

Repository
rDPROV Provenance database
Branch
refactorings
Lint
Lint Skipped
Unit
Unit Tests Skipped
Build Status
Buildable 19355
Build 30013: arc lint + arc unit

Event Timeline

douardda requested review of this revision.Feb 12 2021, 4:31 PM
douardda created this revision.
Harbormaster completed remote builds in B19206: Diff 18098.Feb 12 2021, 4:31 PM
douardda added a child revision: D5073: Refactor the ProvenanceDB classes.Feb 12 2021, 4:32 PM
douardda retitled this revision from Refactor the db scaffolding to use swh.core.db to WIP Refactor the db scaffolding to use swh.core.db.Feb 12 2021, 5:33 PM
douardda updated this revision to Diff 18134.Feb 16 2021, 10:39 AM

moslty rebase on D5080

Harbormaster completed remote builds in B19238: Diff 18134.Feb 16 2021, 10:39 AM
douardda added a parent revision: D5080: Add a pre-commit config file.Feb 16 2021, 10:40 AM
douardda updated this revision to Diff 18138.Feb 16 2021, 12:13 PM

Fix tests and properly initialize the db using swh.core.db in the pytest fixture

Harbormaster completed remote builds in B19242: Diff 18138.Feb 16 2021, 12:13 PM
douardda retitled this revision from WIP Refactor the db scaffolding to use swh.core.db to Refactor the db scaffolding to use swh.core.db.Feb 16 2021, 12:14 PM
douardda added reviewers: aeviso, Reviewers.
douardda updated this revision to Diff 18148.Feb 16 2021, 3:43 PM

rebase and fix tests

Harbormaster completed remote builds in B19252: Diff 18148.Feb 16 2021, 3:43 PM
douardda updated this revision to Diff 18156.Feb 16 2021, 5:33 PM

rebase

Harbormaster completed remote builds in B19260: Diff 18156.Feb 16 2021, 5:33 PM
vlorentz added a subscriber: vlorentz.Feb 18 2021, 12:17 PM
vlorentz added inline comments.
README.md
29

outdated cls?

swh/provenance/__init__.py
21

let's call it "postgres" (we should also rename it that way in swh-storage)

vlorentz added a comment.Feb 18 2021, 12:19 PM

I'm not convinced by the flavor thing. Why not a single schema, but with empty values in loc when it's disabled?

aeviso added a comment.Feb 18 2021, 1:47 PM

I agree with this revision. I only have those few comments/questions above.

swh/provenance/__init__.py
21

I agree "postgres" is more declarative as we can have several "local" db types.

23–26

Does this mean that the default is without path? I think we want it the other way around.

swh/provenance/sql/30-schema.sql
9

Why are the indexes created with the table here and separately in the case of the flavors? Do we have an standard for that?

douardda added a comment.Feb 18 2021, 5:18 PM
In D5072#128933, @vlorentz wrote:

I'm not convinced by the flavor thing. Why not a single schema, but with empty values in loc when it's disabled?

I've kept the logic as it was before... just made it use swh.core

Ir's not even sure the 'without-path' version will stay, so...

douardda added inline comments.Feb 18 2021, 5:20 PM
swh/provenance/__init__.py
21

then I'll change it here when it's done in "core components"

23–26

oh you're right, let me fix this

douardda added inline comments.Feb 18 2021, 5:23 PM
swh/provenance/sql/30-schema.sql
9

I've mainly been lazy there. This is common to both flavors, so it can stay here, but yeas, it would make sense to put it in a dedicated 60-indexes.sql file, since this is generally what's done in other packages.

douardda updated this revision to Diff 18261.Feb 18 2021, 5:49 PM

rebase + default to with_path=True as asked by @aeviso

I've not extracted all the index creations to a dedicated sql file, too tired for that right now...

Harbormaster completed remote builds in B19355: Diff 18261.Feb 18 2021, 5:49 PM
aeviso accepted this revision.Feb 18 2021, 7:28 PM
This revision is now accepted and ready to land.Feb 18 2021, 7:28 PM
Closed by commit rDPROVdabbd5334967: Refactor the database creation code and a test for the cli command (authored by douardda). · Explain WhyFeb 19 2021, 10:01 AM
This revision was automatically updated to reflect the committed changes.
douardda added a commit: rDPROVdabbd5334967: Refactor the database creation code and a test for the cli command.
douardda added a commit: rDPROV5d3ef241e86e: Refactor the db initialization scaffolding to use swh.core.db.

Revision Contents
Changeset List

PathSize
31 lines
1 line
4 lines
swh/
provenance/
12 lines
30 lines
postgresql/
21 lines
postgresql_nopath/
20 lines
sql/
postgresql_nopath/
30-schema.sql
provenance.sql
62 lines
sql/
46 lines
29 lines
3 lines
3 lines
tests/
28 lines
39 lines
9 lines

Diff 18261

View Options

README.md

Loading...
View Options

conftest.py

Loading...
View Options

requirements-swh.txt

Loading...
View Options

swh/provenance/__init__.py

Loading...
View Options

swh/provenance/cli.py

Loading...
View Options

swh/provenance/postgresql/provenance.py

Loading...
View Options

swh/provenance/postgresql/provenance.sql

Loading...
View Options

swh/provenance/postgresql_nopath/provenance.py

Loading...
View Options

swh/provenance/postgresql_nopath/provenance.sql

Loading...
View Options

swh/provenance/sql/30-schema.sql

Loading...
View Options

swh/provenance/sql/35-schema-with-path-flavor.sql

Loading...
View Options

swh/provenance/sql/35-schema-without-path-flavor.sql

Loading...
View Options

swh/provenance/sql/60-indexes-with-path-flavor.sql

Loading...
View Options

swh/provenance/sql/60-indexes-without-path-flavor.sql

Loading...
View Options

swh/provenance/tests/conftest.py

Loading...
View Options

swh/provenance/tests/test_cli.py

Loading...
View Options

swh/provenance/tests/test_conftest.py

Loading...
About · Developer information · Legal