Skip to content
Snippets Groups Projects

Make swh db init and swh db-init behavior much closer to one another

Closed Make swh db init and swh db-init behavior much closer to one another
generated-differential-D3970-source into generated-differential-D3970-target
2 unresolved threads
Closed Nicolas Dandrimont requested to merge generated-differential-D3970-source into generated-differential-D3970-target
2 unresolved threads

While named similarly, these two CLI utilities used to take quite a different approach to doing the same thing.

They now both call out the same initialization function for a given module. The main difference is now the source of information for what databases to initialize: swh db init uses the SWH_CONFIG_FILENAME configuration file, while swh db-init only uses its own command line arguments.

(also sneak in a cosmetic change in a separate commit to db_testing.swh_db_version)

Test Plan

  • reviewed the use of swh db init and swh db-init in all modules and their documentation; All documented usage is still valid.
  • ran the docker initialization with the new swh-core which worked fine

Migrated from D3970 (view on Phabricator)

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Software Heritage Continuous Integration for public repositories
    Software Heritage Continuous Integration for public repositories @swh-public-ci ·

    Build has FAILED

    Patch application report for D3970 (id=13992)

    Rebasing onto 22e77212...

    Current branch diff-target is up to date.
    Changes applied before test
    commit 2c086ddee9f249a0ba2ba8d47c7eff62faf24b98
Author: Nicolas Dandrimont <nicolas@dandrimont.eu>
Date:   Wed Sep 16 19:13:51 2020 +0200

    Make swh db init and swh db-init behavior much closer to one another
    
    While named similarly, these two CLI utilities used to take quite a different
    approach to doing the same thing.
    
    They now both call out the same initialization function for a given module. The
    main difference is now the source of information for what databases to
    initialize: `swh db init` uses the SWH_CONFIG_FILENAME configuration file, while
    `swh db-init` only uses its own command line arguments.

commit 327f98d1860280c9cbde86af14addf183eff112a
Author: Nicolas Dandrimont <nicolas@dandrimont.eu>
Date:   Wed Sep 16 18:57:11 2020 +0200

    Ignore stderr in db_testing.swh_db_version instead of printing it out

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

  • Nicolas Dandrimont
    Nicolas Dandrimont @olasd ·
    Author Maintainer

    Fix flake8 issue that snuck past pre-commit thanks to rebase shenanigans

  • Software Heritage Continuous Integration for public repositories
    Software Heritage Continuous Integration for public repositories @swh-public-ci ·

    Build is green

    Patch application report for D3970 (id=13993)

    Rebasing onto 22e77212...

    Current branch diff-target is up to date.
    Changes applied before test
    commit 6b983c28a3bea0987281cbeb48004c164f55d07c
Author: Nicolas Dandrimont <nicolas@dandrimont.eu>
Date:   Wed Sep 16 19:13:51 2020 +0200

    Make swh db init and swh db-init behavior much closer to one another
    
    While named similarly, these two CLI utilities used to take quite a different
    approach to doing the same thing.
    
    They now both call out the same initialization function for a given module. The
    main difference is now the source of information for what databases to
    initialize: `swh db init` uses the SWH_CONFIG_FILENAME configuration file, while
    `swh db-init` only uses its own command line arguments.

commit 327f98d1860280c9cbde86af14addf183eff112a
Author: Nicolas Dandrimont <nicolas@dandrimont.eu>
Date:   Wed Sep 16 18:57:11 2020 +0200

    Ignore stderr in db_testing.swh_db_version instead of printing it out

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

  • Antoine R. Dumont
    Antoine R. Dumont @ardumont started a thread on the diff
swh/core/cli/db.py
72 74 will initialize the database for the `storage` section using initialization
73 75 scripts from the `swh.storage` package.
74 76 """
75 import subprocess
76
77 77 for modname, cfg in ctx.obj["config"].items():
78 if cfg.get("cls") == "local" and cfg.get("args"):
78 if cfg.get("cls") == "local" and cfg.get("args", {}).get("db"):
  • Antoine R. Dumont
    Antoine R. Dumont @ardumont ·
    Maintainer

    i think we could also have the case of cfg.get("db") return something directly.

    Configuration can be either:

    storage:
    cls: local
    args: 
      db: ...

    covered here.

    or with the "new" abilities from get_storage (iirc):

    storage:
  cls: local
  db: ...
  • Nicolas Dandrimont
    Nicolas Dandrimont @olasd ·
    Author Maintainer

    Sure, this should be fixed in a followup commit.

    I don't think anything actually uses swh db init anyway.

  • Please register or sign in to reply
  • Antoine R. Dumont
    Antoine R. Dumont @ardumont started a thread on the diff
    • swh/core/cli/db.py
    87 conninfo = cfg["args"]["db"]
    88 for sqlfile in sqlfiles:
    89 subprocess.check_call(
    90 [
    91 "psql",
    92 "--quiet",
    93 "--no-psqlrc",
    94 "-v",
    95 "ON_ERROR_STOP=1",
    96 "-d",
    97 conninfo,
    98 "-f",
    99 sqlfile,
    100 ]
    101 )
    88 click.secho(
  • Antoine R. Dumont
    Antoine R. Dumont @ardumont ·
    Maintainer

    lgtm

    one remark/question above.

    Thanks.

  • Antoine R. Dumont
    Antoine R. Dumont @ardumont ·
    Maintainer

    Merge request was accepted

  • Antoine R. Dumont approved this merge request

    approved this merge request

  • Nicolas Dandrimont
    Nicolas Dandrimont @olasd ·
    Author Maintainer

    Merge request was merged

  • Nicolas Dandrimont closed

    closed

    • Please register or sign in to reply