Page MenuHomeSoftware Heritage
Differential D3997

Add support for database flavors to `swh db-init`
ClosedPublic
Actions

Authored by olasd on Sep 18 2020, 5:46 PM.

Details

Reviewers
douardda
ardumont
Commits
rDCORE7c117c3dd010: Quote database names in `CREATE DATABASE` statements
rDCORE255e0719127a: Support using a full DSN or a single database name in `swh db create`
rDCORE083fdb1e9b61: Add support for database flavors to `swh db init`
Summary

This adds a "flavor initialization step" to the initialization of the database,
after the execution of the sql file named *-flavor.sql.

I've added a few sanity checks to warn if one of the underlying assumptions for
this feature breaks.

Test Plan

called swh db-init with a bunch of combinations of valid and invalid
flavors to check the behavior.

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

olasd created this revision.Sep 18 2020, 5:46 PM
swh-public-ci added a comment.Sep 18 2020, 5:48 PM

Build is green

Patch application report for D3997 (id=14089)

Could not rebase; Attempt merge onto 28d61c8cf1...

Updating 28d61c8..0610c95
Fast-forward
 swh/core/cli/db.py              | 60 +++++++++++++++++++++++-----------
 swh/core/db/db_utils.py         | 71 ++++++++++++++++++++++++++++++++++-------
 swh/core/db/tests/db_testing.py | 37 ---------------------
 tox.ini                         |  2 +-
 4 files changed, 101 insertions(+), 69 deletions(-)
Changes applied before test
commit 0610c9555e78781c1696bca86462d8a9ab4cbb92
Author: Nicolas Dandrimont <nicolas@dandrimont.eu>
Date:   Fri Sep 18 16:16:51 2020 +0200

    Add support for database flavors to `swh db-init`
    
    This adds a "flavor initialization step" to the initialization of the database,
    after the execution of the sql file named *-flavor.sql.
    
    I've added a few sanity checks to warn if one of the underlying assumptions for
    this feature breaks.

commit e5312bdaa865565d1a6c88ddcdc2ffabfa0bb70c
Author: Nicolas Dandrimont <nicolas@dandrimont.eu>
Date:   Fri Sep 18 15:43:27 2020 +0200

    Reimplement swh_db_version with psycopg2
    
    This allows us to reuse an existing connection if needed, and to be more
    specific which errors we ignore or not.

commit 30be93846340bb289f9e780c5b0d6c50dfe8b506
Author: Nicolas Dandrimont <nicolas@dandrimont.eu>
Date:   Fri Sep 18 15:42:24 2020 +0200

    Pin black to the same version as the pre-commit hook

commit b63d7f432caeb9d75da6151fb20f3764beec7f80
Author: Nicolas Dandrimont <nicolas@dandrimont.eu>
Date:   Fri Sep 18 14:39:50 2020 +0200

    Fix license statements on db_utils.py
    
    When the code was migrated from swh.storage, some other ancillary functions were
    merged into this file. So bits of it are under the LGPLv3 (the code pulled from
    psycopg2) and bits are under the GPLv3 (the code we wrote).

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

Harbormaster completed remote builds in B15356: Diff 14089.Sep 18 2020, 5:48 PM
douardda added a comment.Sep 21 2020, 10:25 AM

I don't understand how this is supposed to work.

How does the flavor "name" is taken in consideration? Is it supposed to select "flavored" versions of sql files? What's the pattern ? As written, I don't understand how the flavor "name" is used besides being written the dbflavor table.

douardda added a comment.Sep 21 2020, 11:56 AM
In D3997#98758, @douardda wrote:

I don't understand how this is supposed to work.

How does the flavor "name" is taken in consideration? Is it supposed to select "flavored" versions of sql files? What's the pattern ? As written, I don't understand how the flavor "name" is used besides being written the dbflavor table.

Oh now I get it (after reading D3981)

It's the presence of a xxx-flavor.sql that triggers the insertion/update of the flavor given as cli argument in the dbflavor table, which then specify how the other sql files are executed (thx to /if statements in there).

douardda added a comment.Sep 21 2020, 11:57 AM
In D3997#98782, @douardda wrote:
In D3997#98758, @douardda wrote:

I don't understand how this is supposed to work.

How does the flavor "name" is taken in consideration? Is it supposed to select "flavored" versions of sql files? What's the pattern ? As written, I don't understand how the flavor "name" is used besides being written the dbflavor table.

Oh now I get it (after reading D3981)

It's the presence of a xxx-flavor.sql that triggers the insertion/update of the flavor given as cli argument in the dbflavor table, which then specify how the other sql files are executed (thx to /if statements in there).

This really worth a comment/explanation somewhere.

douardda accepted this revision.Sep 21 2020, 2:25 PM
This revision is now accepted and ready to land.Sep 21 2020, 2:25 PM
ardumont accepted this revision.Sep 21 2020, 2:40 PM
olasd updated this revision to Diff 14175.Sep 23 2020, 2:07 PM

Rebase and fixup

  • Add support for database flavors to swh db init
  • Support using a full DSN or a single database name in swh db create
  • Quote database names in CREATE DATABASE statements
swh-public-ci added a comment.Sep 23 2020, 2:08 PM

Build is green

Patch application report for D3997 (id=14175)

Rebasing onto b5d607989b...

Current branch diff-target is up to date.
Changes applied before test
commit 899e56b8ff933097d569b45583c9457d007fecad
Author: Nicolas Dandrimont <nicolas@dandrimont.eu>
Date:   Wed Sep 23 14:05:34 2020 +0200

    Quote database names in `CREATE DATABASE` statements
    
    This allows us to create databases with names containing dashes, such as the
    ones advertised in the documentation of the `swh db create` command.

commit 15a385fe9cf2aa77d07548dbed27680151c8c1e3
Author: Nicolas Dandrimont <nicolas@dandrimont.eu>
Date:   Wed Sep 23 14:04:37 2020 +0200

    Support using a full DSN or a single database name in `swh db create`
    
    This matches the functionality advertised as examples in the documentation for
    the command.

commit 5a11786901f1b275ca081ca1a4671d6efdfaafa6
Author: Nicolas Dandrimont <nicolas@dandrimont.eu>
Date:   Fri Sep 18 16:16:51 2020 +0200

    Add support for database flavors to `swh db init`
    
    This adds a "flavor initialization step" to the initialization of the database,
    after the execution of the sql file named *-flavor.sql.
    
    This adds a few sanity checks to warn if one of the underlying assumptions for
    this feature breaks.

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

Harbormaster completed remote builds in B15439: Diff 14175.Sep 23 2020, 2:08 PM
douardda added a comment.Sep 23 2020, 2:20 PM

LGTM, just a couple of wishes (and we are not even close to xmas!)

swh/core/cli/db.py
96

not a blocker, but would it be possible to have this a click.Choice option?

132

shouldn't the "already exists" message also print the flavor?

olasd added inline comments.Sep 23 2020, 2:58 PM
swh/core/cli/db.py
96

I wish it was possible, but in general it depends on which database you're trying to initialize, and you'd have to parse their SQL files to know which flavors exist.

The function does:

  • fail if there's flavor support but the flavor you're requesting doesn't exist
  • warn if there's no flavor support for the module you're initializing, but you requested a flavor
132

Yeah, it should. This means adding a new function to "get" the database flavor, and having an even larger populate_database_... return type, which is annoying and why I didn't do it initially.

olasd updated this revision to Diff 14186.Sep 23 2020, 4:13 PM

Recall the database flavor in swh db init

olasd marked an inline comment as done.Sep 23 2020, 4:13 PM
swh-public-ci added a comment.Sep 23 2020, 4:14 PM

Build is green

Patch application report for D3997 (id=14186)

Rebasing onto b5d607989b...

Current branch diff-target is up to date.
Changes applied before test
commit 7c117c3dd010b6b8f44087a0b04cab815a4d087f
Author: Nicolas Dandrimont <nicolas@dandrimont.eu>
Date:   Wed Sep 23 14:05:34 2020 +0200

    Quote database names in `CREATE DATABASE` statements
    
    This allows us to create databases with names containing dashes, such as the
    ones advertised in the documentation of the `swh db create` command.

commit 255e0719127abec2ff24328a27508febd52ba5a9
Author: Nicolas Dandrimont <nicolas@dandrimont.eu>
Date:   Wed Sep 23 14:04:37 2020 +0200

    Support using a full DSN or a single database name in `swh db create`
    
    This matches the functionality advertised as examples in the documentation for
    the command.

commit 083fdb1e9b61a69b9ba94e31574ec80f0025478f
Author: Nicolas Dandrimont <nicolas@dandrimont.eu>
Date:   Fri Sep 18 16:16:51 2020 +0200

    Add support for database flavors to `swh db init`
    
    This adds a "flavor initialization step" to the initialization of the database,
    after the execution of the sql file named *-flavor.sql.
    
    This adds a few sanity checks to warn if one of the underlying assumptions for
    this feature breaks.

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

Harbormaster completed remote builds in B15451: Diff 14186.Sep 23 2020, 4:14 PM
Closed by commit rDCORE083fdb1e9b61: Add support for database flavors to `swh db init` (authored by olasd). · Explain WhySep 23 2020, 4:16 PM
This revision was automatically updated to reflect the committed changes.
olasd added a commit: rDCORE083fdb1e9b61: Add support for database flavors to `swh db init`.
olasd added a commit: rDCORE255e0719127a: Support using a full DSN or a single database name in `swh db create`.
olasd added a commit: rDCORE7c117c3dd010: Quote database names in `CREATE DATABASE` statements.

Revision Contents
Changeset List

PathSize
swh/
core/
cli/
90 lines
db/
88 lines

Diff 14188

View Options

swh/core/cli/db.py

Loading...
View Options

swh/core/db/db_utils.py

Loading...
About · Developer information · Legal