db.BaseDb: Propose default get_current_version method implementation
AbandonedPublic
Actions

Authored by ardumont on Jun 2 2022, 5:21 PM.

Details

Reviewers

None

Group Reviewers

Reviewers

Maniphest Tasks

T4228: scrubber: Investigate the apparent lock (staging)

Summary

This:

documents what this method is about in the code (only in documentation so far)
adds a missing test on the cli swh db version to check this behaves appropriately
will allow to clean up duplicated code in existing backends (storage, indexer) [2] [3]
will allow to fix the backends which are missing the appropriate methods (scheduler

[4], scrubber, vault, see [1]'s description for details...)

Related to T4228
Depends on D7943 (revert of [1])
This is the proper fix instead of [1].

[1] D7907

[2] https://forge.softwareheritage.org/source/swh-storage/browse/master/swh/storage/postgresql/storage.py$208-211

[3] https://forge.softwareheritage.org/source/swh-indexer/browse/master/swh/indexer/storage/__init__.py$155-157

[4] https://forge.softwareheritage.org/source/swh-scheduler/browse/master/swh/scheduler/backend.py$76-78

Diff Detail

Repository

rDCORE Foundations and core functionalities

Branch

revert-db-migration

Lint

No Linters Available

Unit

No Unit Test Coverage

Build Status

Buildable 29712
Build 46434: Phabricator diff pipeline on jenkins	Jenkins console · Jenkins
Build 46433: arc lint + arc unit

Event Timeline

ardumont created this revision.Jun 2 2022, 5:21 PM

Herald added a reviewer: Reviewers. · View Herald TranscriptJun 2 2022, 5:21 PM

ardumont edited the summary of this revision. (Show Details)Jun 2 2022, 5:21 PM

ardumont added a parent revision: D7943: Revert "cli.db: Use attribute current_version instead of undeclared getter".

Build is green

Patch application report for D7949 (id=28629)

Could not rebase; Attempt merge onto e1a1d84eb4...

Updating e1a1d84..303023b
Fast-forward
 docs/db.rst                   |  4 ++--
 swh/core/cli/db.py            | 14 ++++++++------
 swh/core/db/__init__.py       | 11 +++++++++++
 swh/core/db/tests/conftest.py |  4 ++--
 swh/core/db/tests/test_cli.py | 24 +++++++++++++++++++++++-
 5 files changed, 46 insertions(+), 11 deletions(-)

Changes applied before test

commit 303023b44beeb868acae5010d7b9e575f8573611
Author: Antoine R. Dumont (@ardumont) <antoine.romain.dumont@gmail.com>
Date:   Thu Jun 2 17:14:08 2022 +0200

    db.BaseDb: Propose default get_current_version method implementation
    
    This:
    - documents what this method is about in the code (only in documentation so far)
    - adds a missing test on the cli `swh db version` to check this behaves appropriately
    - will allow to clean up duplicated code in existing backends (storage, indexer)
    - will allow to fix the backends which are missing the appropriate methods (scrubber,
    scheduler, ...)
    
    Related to T4228

commit 1096de5a182968cebbbd98d921814fc332095710
Author: Antoine R. Dumont (@ardumont) <antoine.romain.dumont@gmail.com>
Date:   Thu Jun 2 11:40:00 2022 +0200

    Revert "cli.db: Use attribute current_version instead of undeclared getter"
    
    This reverts commit 5cda0ca626010cccc7e7f3f73fbfb1b20702ac40. which is not the proper
    fix.
    
    The proper fix plan becomes:
    - fix documentation to explicit the get_current_version intention
    
    - either fix swh-scrubber or provide a default implementation in BaseDb to actually
    expose that get_current_version (that can then be overriden if something specific needs
    to be developed for a new datastore).
    
    Related to T4284

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

Harbormaster completed remote builds in B29712: Diff 28629.Jun 2 2022, 5:23 PM

ardumont requested review of this revision.Jun 2 2022, 5:23 PM

ardumont edited the summary of this revision. (Show Details)Jun 2 2022, 5:23 PM

ardumont edited the summary of this revision. (Show Details)Jun 2 2022, 5:28 PM

ardumont mentioned this in D7943: Revert "cli.db: Use attribute current_version instead of undeclared getter".Jun 2 2022, 5:31 PM

Not the proper fix

Although some stuff are worth keeping, notably the tests on the missing coverage cli (i'll do it in another diff).

ardumont mentioned this in T4305: Aligh swh backends for migration tools to work.Jun 3 2022, 11:33 AM

ardumont mentioned this in D7968: Add missing coverage on `swh db version` cli.Jun 8 2022, 10:46 AM