Page MenuHomeSoftware Heritage

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

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

Details

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

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.

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.

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).

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.

This revision is now accepted and ready to land.Sep 21 2020, 2:25 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

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.

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?

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.

Recall the database flavor in swh db init

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.