Page MenuHomeSoftware Heritage
Differential D3385

origin_visits/get_origin_visit: Improve default visit picking strategy
ClosedPublic
Actions

Authored by anlambert on Jun 30 2020, 6:37 PM.

Details

Reviewers
ardumont
Group Reviewers
Reviewers
Commits
rDWAPPS76d9162807e0: origin_visits/get_origin_visit: Improve default visit picking strategy
Summary

When no visit info (timestamp, id or snapshot) is provided to that function
return the last full visit with a valid snapshot or the last partial visit
with a valid snapshot otherwise.

This fixes a browsing issue when an origin is no more available but swh keeps
visiting it afterwards.

For instance the origin https://github.com/baifanwudi/PythonCode has been removed
since the first swh visit. Currently, when trying to browse it, we get
and empty snapshot view.
After that change, the last full visit with non empty snapshot will be displayed instead.

Diff Detail

Repository
rDWAPPS Web applications
Branch
improve-get-origin-visit
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 13248
Build 20231: Phabricator diff pipeline on jenkinsJenkins console · Jenkins
Build 20230: arc lint + arc unit

Event Timeline

anlambert created this revision.Jun 30 2020, 6:37 PM
Herald added a reviewer: Reviewers. · View Herald TranscriptJun 30 2020, 6:37 PM
swh-public-ci added a comment.Jun 30 2020, 6:47 PM

Build is green

Patch application report for D3385 (id=12012)

Rebasing onto f433684afd...

Current branch diff-target is up to date.
Changes applied before test
commit 774c25ab2e227ff207f8eed55e128935afff53f3
Author: Antoine Lambert <antoine.lambert@inria.fr>
Date:   Tue Jun 30 18:25:58 2020 +0200

    origin_visits/get_origin_visit: Improve default visit picking strategy
    
    When no visit info (timestamp, id or snapshot) is provided to that function
    return the last full visit with a valid snapshot or the last partial visit
    with a valid snapshot otherwise.
    
    This fixes a browsing issue when an origin is no more available but swh keeps
    visiting it afterwards.

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

Harbormaster completed remote builds in B13231: Diff 12012.Jun 30 2020, 6:47 PM
anlambert edited the summary of this revision. (Show Details)Jul 1 2020, 1:48 AM
anlambert updated this revision to Diff 12017.Jul 1 2020, 11:35 AM

Rebase

swh-public-ci added a comment.Jul 1 2020, 11:47 AM

Build is green

Patch application report for D3385 (id=12017)

Rebasing onto c2726ac124...

Current branch diff-target is up to date.
Changes applied before test
commit 8f66f393a0d770338576ffc44473d4c4477f05a6
Author: Antoine Lambert <antoine.lambert@inria.fr>
Date:   Tue Jun 30 18:25:58 2020 +0200

    origin_visits/get_origin_visit: Improve default visit picking strategy
    
    When no visit info (timestamp, id or snapshot) is provided to that function
    return the last full visit with a valid snapshot or the last partial visit
    with a valid snapshot otherwise.
    
    This fixes a browsing issue when an origin is no more available but swh keeps
    visiting it afterwards.

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

Harbormaster completed remote builds in B13235: Diff 12017.Jul 1 2020, 11:47 AM
ardumont added a subscriber: ardumont.Jul 1 2020, 2:23 PM

Looks good, some remarks above though (proposal to improve coverage).

swh/web/common/origin_visits.py
166

Having the snapshot-id set does not mean that it's necessarily valid currently (unfortunately) [1]

[1] D3322

swh/web/tests/common/test_origin_visits.py
229

That does not really test the second part (fallback on partial visit with a snapshot ;).

For that maybe make a first pass on your local visits list without the visit id 3.
That should then found the partial visit with id 6 (if i'm not mistaken).

Then append visit id 3 to your list.
Trigger a search again which should be the current scenario ;)

Also, i don't know if the list is already ordered or not.

What do you think?

ardumont added inline comments.Jul 1 2020, 2:34 PM
swh/web/tests/common/test_origin_visits.py
229

Also, i don't know if the list is already ordered or not.

I mean it apparently is as you reverse it...
No idea if the order should be enforced "more" by sorting on the key "date" there...

Also, it kinda feel redundant with the
`swh.storage.algos.origin_get_latest_visit_status(require_snapshot=True,
allowed_statuses=["full", "partial"])` somehow (<- except you don't have
priority granularity you coded here regarding the status, full first, then
partial as fallback...)

anlambert planned changes to this revision.Jul 1 2020, 2:59 PM

This needs more work in order to reuse utility functions from storage and rewrite tests using hypothesis.

swh/web/tests/common/test_origin_visits.py
229

Indeed, this is the same processing but that visit picking strategy was in place since a long time in swh-web and the origin_get_latest_visit_status function has landed recently in storage so I did not really have it in mind.

I think I might be able to use it in the get_origin_visit function.

229

I think the best option here is to rewrite the tests using hypothesis to avoid hardcoding visits data and mocking functions.

anlambert updated this revision to Diff 12029.Jul 1 2020, 6:15 PM

Update:

  • Use new swh.storage.algos.origin.origin_get_latest_visit_status utility function in revised get_origin_visit implementation
  • Rewrite tests using swh models and hypothesis
  • Update some function documentation
swh-public-ci added a comment.Jul 1 2020, 6:26 PM

Build is green

Patch application report for D3385 (id=12029)

Rebasing onto b8eb774184...

Current branch diff-target is up to date.
Changes applied before test
commit 8df76362f8a0a6e14ba49d551ca0e5b85b6332f9
Author: Antoine Lambert <antoine.lambert@inria.fr>
Date:   Tue Jun 30 18:25:58 2020 +0200

    origin_visits/get_origin_visit: Improve default visit picking strategy
    
    When no visit info (timestamp, id or snapshot) is provided to that function
    return the last full visit with a valid snapshot or the last partial visit
    with a valid snapshot otherwise.
    
    This fixes a browsing issue when an origin is no more available but swh keeps
    visiting it afterwards.
    
    Use new swh.storage.algos.origin.origin_get_latest_visit_status utility
    function in the revised get_origin_visit_implementation.

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

Harbormaster completed remote builds in B13248: Diff 12029.Jul 1 2020, 6:26 PM
ardumont accepted this revision.Jul 1 2020, 8:21 PM

nice ;)

This revision is now accepted and ready to land.Jul 1 2020, 8:21 PM
anlambert updated this revision to Diff 12031.Jul 2 2020, 11:24 AM

Update: Improve tests implementation

swh-public-ci added a comment.Jul 2 2020, 11:35 AM

Build is green

Patch application report for D3385 (id=12031)

Rebasing onto b8eb774184...

Current branch diff-target is up to date.
Changes applied before test
commit 76d9162807e06e8ab86d2b9f517aad343df22845
Author: Antoine Lambert <antoine.lambert@inria.fr>
Date:   Tue Jun 30 18:25:58 2020 +0200

    origin_visits/get_origin_visit: Improve default visit picking strategy
    
    When no visit info (timestamp, id or snapshot) is provided to that function
    return the last full visit with a valid snapshot or the last partial visit
    with a valid snapshot otherwise.
    
    This fixes a browsing issue when an origin is no more available but swh keeps
    visiting it afterwards.
    
    Use new swh.storage.algos.origin.origin_get_latest_visit_status utility
    function in the revised get_origin_visit_implementation.

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

Harbormaster completed remote builds in B13251: Diff 12031.Jul 2 2020, 11:35 AM
Closed by commit rDWAPPS76d9162807e0: origin_visits/get_origin_visit: Improve default visit picking strategy (authored by anlambert). · Explain WhyJul 2 2020, 11:39 AM
This revision was automatically updated to reflect the committed changes.
anlambert added a commit: rDWAPPS76d9162807e0: origin_visits/get_origin_visit: Improve default visit picking strategy.

Revision Contents
Changeset List

PathSizeCoverage (All)Coverage (Touched)
swh/
web/
common/
105 lines88%
Loading...
23 lines91%
Loading...
tests/
common/
157 lines100%
Loading...

Diff 12029

View Options

swh/web/common/origin_visits.py

Loading...
View Options

swh/web/common/service.py

Loading...
View Options

swh/web/tests/common/test_origin_visits.py

Loading...
About · Developer information · Legal