Page MenuHomeSoftware Heritage
Differential D8066

Add the 'flavor' argument to db_utils.initialize_database_for_module()
ClosedPublic
Actions

Authored by douardda on Jul 1 2022, 3:01 PM.

Details

Reviewers
ardumont
Group Reviewers
Reviewers
Commits
rDCOREb805b3106fb9: Add the 'flavor' argument to db_utils.initialize_database_for_module()
Summary

so one can create database (e.g. for unit tests) of given flavor.

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

douardda created this revision.Jul 1 2022, 3:01 PM
Herald added a reviewer: Reviewers. · View Herald TranscriptJul 1 2022, 3:01 PM
swh-public-ci added a comment.Jul 1 2022, 3:03 PM

Build is green

Patch application report for D8066 (id=29107)

Rebasing onto 87e7afab11...

Current branch diff-target is up to date.
Changes applied before test
commit 0d29492d6022f5c574d7714164937d955eacc242
Author: David Douard <david.douard@sdfa3.org>
Date:   Fri Jul 1 14:58:31 2022 +0200

    Add the 'flavor' argument to db_utils.initialize_database_for_module()
    
    so one can create database (e.g. for unit tests) of given flavor.

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

Harbormaster completed remote builds in B30199: Diff 29107.Jul 1 2022, 3:03 PM
douardda requested review of this revision.Jul 1 2022, 3:03 PM
douardda added a child revision: D8067: Add a Storage.flavor property to the postgresql backend.Jul 1 2022, 3:04 PM
ardumont added a subscriber: ardumont.Jul 4 2022, 9:55 AM

lgtm but it's missing the test case around it. It should not be too hard to add a scenario which lifts the flavor parameter, shouldn't it?

douardda added a comment.Jul 4 2022, 11:00 AM
In D8066#209725, @ardumont wrote:

lgtm but it's missing the test case around it. It should not be too hard to add a scenario which lifts the flavor parameter, shouldn't it?

possibly, but I had it tested from swh-storage (D8067) (but yeah a at least basic test in core would be better)

ardumont added a comment.Jul 4 2022, 11:43 AM

lgtm but it's missing the test case around it. It should not be too hard to add a

scenario which lifts the flavor parameter, shouldn't it? possibly, but I had it tested
from swh-storage (D8067)

ah yes, and i did not see it since it needs a release from swh.core with it (test
failing hence that diff is in draft).

(but yeah a at least basic test in core would be better)

yub yub

douardda updated this revision to Diff 29163.Jul 5 2022, 10:26 AM

add tests + rebase

swh-public-ci added a comment.Jul 5 2022, 10:28 AM

Build is green

Patch application report for D8066 (id=29163)

Rebasing onto d0e7f5a10e...

Current branch diff-target is up to date.
Changes applied before test
commit bcdf4cc7bdf1c15d74e766b9d055e70bb902ceb0
Author: David Douard <david.douard@sdfa3.org>
Date:   Fri Jul 1 14:58:31 2022 +0200

    Add the 'flavor' argument to db_utils.initialize_database_for_module()
    
    so one can create database (e.g. for unit tests) of given flavor.

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

Harbormaster completed remote builds in B30253: Diff 29163.Jul 5 2022, 10:28 AM
douardda updated this revision to Diff 29164.Jul 5 2022, 10:36 AM

fix the test (op. priority...)

swh-public-ci added a comment.Jul 5 2022, 10:38 AM

Build is green

Patch application report for D8066 (id=29164)

Rebasing onto d0e7f5a10e...

Current branch diff-target is up to date.
Changes applied before test
commit b805b3106fb9d4e0385a21c49fc4ff4be6584889
Author: David Douard <david.douard@sdfa3.org>
Date:   Fri Jul 1 14:58:31 2022 +0200

    Add the 'flavor' argument to db_utils.initialize_database_for_module()
    
    so one can create database (e.g. for unit tests) of given flavor.

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

Harbormaster completed remote builds in B30254: Diff 29164.Jul 5 2022, 10:38 AM
ardumont accepted this revision.Jul 5 2022, 2:07 PM

lgtm

one question on the last assertion test though.

swh/core/db/tests/test_db_utils.py
189

Oh nice, I guess that combines the parametrization.

207

or "default"?

This revision is now accepted and ready to land.Jul 5 2022, 2:07 PM
douardda added inline comments.Jul 5 2022, 2:19 PM
swh/core/db/tests/test_db_utils.py
207

if the flavor param is None, then no flavor is specified at swh db init time, but the default value will be 'default' according the sql code in 15-flavor.sql files (stolen from swh-storage).

Closed by commit rDCOREb805b3106fb9: Add the 'flavor' argument to db_utils.initialize_database_for_module() (authored by douardda). · Explain WhyJul 5 2022, 2:19 PM
This revision was automatically updated to reflect the committed changes.
douardda added a commit: rDCOREb805b3106fb9: Add the 'flavor' argument to db_utils.initialize_database_for_module().
ardumont added inline comments.Jul 5 2022, 2:20 PM
swh/core/db/tests/test_db_utils.py
207

makes sense, thx.

Revision Contents
Changeset List

PathSize
swh/
core/
db/
6 lines
tests/
data/
cli/
sql/
22 lines
cli_new/
sql/
22 lines
4 lines
27 lines

Diff 29167

View Options

swh/core/db/db_utils.py

Loading...
View Options

swh/core/db/tests/data/cli/sql/15-flavor.sql

Loading...
View Options

swh/core/db/tests/data/cli_new/sql/15-flavor.sql

Loading...
View Options

swh/core/db/tests/test_cli.py

Loading...
View Options

swh/core/db/tests/test_db_utils.py

Loading...
About · Developer information · Legal