Page MenuHomeSoftware Heritage

swh-db-init: Make the db initialization idempotent
ClosedPublic

Authored by ardumont on Dec 14 2018, 12:03 PM.

Diff Detail

Repository
rDCORE Foundations and core functionalities
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.Dec 14 2018, 12:03 PM
ardumont updated this revision to Diff 2603.Dec 14 2018, 12:07 PM

Fix print statements

ardumont marked 2 inline comments as done.Dec 14 2018, 12:21 PM
ardumont added inline comments.
swh/core/cli.py
28 ↗(On Diff #2603)

Oh yes, this can be removed, it tries to create the db each time without breaking if it cannot now.

80 ↗(On Diff #2603)

A priori the directive to include the sql upgrade scripts are there so the swh.* pypi service artifact should contain the necessary sql scripts for it.

ardumont updated this revision to Diff 2606.Dec 14 2018, 1:25 PM
  • swh.core.cli: Remove no longer used --create/--no-create flags
ardumont marked an inline comment as done.Dec 14 2018, 1:26 PM
ardumont added inline comments.
swh/core/cli.py
28 ↗(On Diff #2603)

done

vlorentz accepted this revision.Dec 14 2018, 1:33 PM
vlorentz added inline comments.
swh/core/cli.py
76 ↗(On Diff #2603)

redundant test

swh/core/tests/db_testing.py
16–40

Can't we use psycopg2 for that?

81–82

Starting with Python 3.5, you can do this instead:

subprocess.run(['createdb', dbname], check=check)
This revision is now accepted and ready to land.Dec 14 2018, 1:33 PM
ardumont marked 3 inline comments as done.Dec 14 2018, 1:36 PM
ardumont added inline comments.
swh/core/cli.py
76 ↗(On Diff #2603)

It's to have the db version when we are done migrating the schema.
The log at the end of the file will display at what version we are.

swh/core/tests/db_testing.py
16–40

I tried but then that would mean i'd need to change the input.
As dbname is not enough.

At the moment, we can both use dbname and a string like 'service=swh' (providing you have the right .pgass and .pg_service.conf installed).

81–82

So true!

ardumont marked an inline comment as done.Dec 14 2018, 1:37 PM
ardumont added inline comments.
swh/core/cli.py
76 ↗(On Diff #2603)

s/migrating/initializing/

ardumont updated this revision to Diff 2607.Dec 14 2018, 1:40 PM
  • db_testing: Simplify pg_createdb
This revision was automatically updated to reflect the committed changes.
vlorentz added inline comments.Dec 14 2018, 1:43 PM
swh/core/cli.py
76 ↗(On Diff #2603)

I know. But since not db_version is always True at this point, you can replace this:

if not db_version:
    db_version = swh_db_version(db_name)

with this:

db_version = swh_db_version(db_name)
ardumont marked an inline comment as done.Dec 14 2018, 1:44 PM
ardumont added inline comments.
swh/core/cli.py
76 ↗(On Diff #2603)

Right!

vlorentz added a comment.EditedDec 14 2018, 1:44 PM

What happened to this diff? changes to swh/core/cli.py disappeared

What happened to this diff? changes to swh/core/cli.py disappeared?

I pushed the commits.
So that closes it (good).
and then somehow that screws everything in the diff view (bad).

ardumont marked an inline comment as done.Dec 17 2018, 10:05 AM
ardumont added inline comments.
swh/core/tests/db_testing.py
16–40

What i meant was, in the current context it's used (swh.core.cli), this is fine.
If we want to actually implement using psycopg2, we will have to draw a not so simple refactoring (including the swh.core.cli which is not tested yet).