Stop relying on origin ids unless necessary.
ClosedPublic
Actions

Authored by vlorentz on Jul 4 2019, 5:03 PM.

Details

Reviewers

anlambert

Group Reviewers

Reviewers

Maniphest Tasks

T1816: Make all clients of swh-storage use origin URL as identifier and use visit types instead of origin types

Commits

rDWAPPS6eb1a9bd7ee9: Stop relying on origin ids unless necessary.

Summary

This completely stops using origin ids, except when an endpoint that depends
on origin ids is used.

Related: D1690

Test Plan

# Test with origin ids
pytest --hypothesis-profile=swh-web-fast swh/web/

# Disable origin ids
cd ../swh-storage/
arc patch D1690
cd ../swh-web/

# Test without origin ids (and disable tests that require origin ids)
SWH_STORAGE_IN_MEMORY_ENABLE_ORIGIN_IDS=false pytest --hypothesis-profile=swh-web-fast swh/web/ -m "not origin_id"

Diff Detail

Repository

rDWAPPS Web applications

Lint

Automatic diff as part of commit; lint not applicable.

Unit

Automatic diff as part of commit; unit tests not applicable.

Event Timeline

vlorentz created this revision.Jul 4 2019, 5:03 PM

Herald added a reviewer: Reviewers. · View Herald TranscriptJul 4 2019, 5:03 PM

vlorentz edited the test plan for this revision. (Show Details)Jul 4 2019, 5:04 PM

vlorentz edited the test plan for this revision. (Show Details)

vlorentz edited the summary of this revision. (Show Details)Jul 4 2019, 5:06 PM

Build is green
See https://jenkins.softwareheritage.org/job/DWAPPS/job/tox/581/ for more details.

Harbormaster completed remote builds in B6695: Diff 5672.Jul 4 2019, 5:06 PM

vlorentz mentioned this in D1690: Add an env var to the in-mem storage to disable origin ids..Jul 4 2019, 5:06 PM

vlorentz added a task: T1816: Make all clients of swh-storage use origin URL as identifier and use visit types instead of origin types.Jul 4 2019, 5:48 PM

vlorentz mentioned this in T1816: Make all clients of swh-storage use origin URL as identifier and use visit types instead of origin types.

sed "s/ENABLE_ORIGIN_IDS = True/ENABLE_ORIGIN_IDS = False/" swh/storage/in_memory.py

why don't you make it a configuration option?

use a config var instead of sed

vlorentz edited the test plan for this revision. (Show Details)Jul 8 2019, 10:59 AM

Build is green
See https://jenkins.softwareheritage.org/job/DWAPPS/job/tox/585/ for more details.

Harbormaster completed remote builds in B6711: Diff 5690.Jul 8 2019, 11:02 AM

Looks good to me. FYI, pytest is printing the following warning:

PytestUnknownMarkWarning: Unknown pytest.mark.origin_id - is this a typo?  You can register custom marks to avoid this warning - for details, see https://docs.pytest.org/en/latest/mark.html

The new marker needs to be explicitely declared in the pytest.ini file to remove the warning:

markers =
  origin_id: execute tests using an origin id (deselect with '-m "not origin_id"')

swh/web/common/service.py

957

Here, I would prefer to put the test inside parenthesis instead of using backslashes:

if ('origin_url' in revision and
            'branch_name' in revision and
            'ts' in revision and
            'sha1_git' in revision):
        return lookup_revision_with_context_by(revision['origin_url'],
                                               revision['branch_name'],
                                               revision['ts'],
                                               revision['sha1_git'],
                                               limit)