Paths

Table of Contentst

Differential D6641

browse/snapshot_context: Handle snapshot containing only releases
ClosedPublic
Actions

Authored by anlambert on Nov 16 2021, 11:42 AM.

Details

Reviewers

vlorentz

Group Reviewers

Reviewers

Maniphest Tasks

T3724: Browsing snapshot with only releases does not show anything in the webapp

Summary

This fixes observed errors when trying to browse a snapshot containing
only releases.

Releases targeting directories are also now properly handled.

Related to T3724

Diff Detail

Repository

rDWAPPS Web applications

Branch

browse-snapshot-releases-only

Lint

No Linters Available

Unit

No Unit Test Coverage

Build Status

Buildable 25030
Build 39115: Phabricator diff pipeline on jenkins	Jenkins console · Jenkins
Build 39114: arc lint + arc unit

Event Timeline

anlambert created this revision.Nov 16 2021, 11:42 AM

Herald added a reviewer: Reviewers. · View Herald TranscriptNov 16 2021, 11:42 AM

Build is green

Patch application report for D6641 (id=24139)

Rebasing onto e2659cfed5...

Current branch diff-target is up to date.

Changes applied before test

commit 9725d7135b2e7fab7074d1cd20c2cf33cfef9e6d
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Mon Nov 15 16:36:29 2021 +0100

    browse/snapshot_context: Handle snapshot containing only releases
    
    Related to T3724

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

Harbormaster completed remote builds in B25019: Diff 24139.Nov 16 2021, 11:56 AM

anlambert requested review of this revision.Nov 16 2021, 11:56 AM

vlorentz added a subscriber: vlorentz.Nov 16 2021, 12:27 PM

vlorentz added inline comments.

swh/web/browse/snapshot_context.py
602	Wouldn't it make sense to use the last one? If releases are named after versions, the last one is more likely the be the most recent, which is the "best" one
swh/web/tests/browse/test_snapshot_context.py
11–12	for consistency with other packages. (we really need to get rid of this.....)

anlambert added inline comments.Nov 16 2021, 1:23 PM

swh/web/browse/snapshot_context.py
602	Indeed it makes sense, will update.
swh/web/tests/browse/test_snapshot_context.py
11–12	ack, yes it is so confusing, not the first time I encounter that issue.

Update:

handle @vlorentz comments
ensure all get_snapshot_context code is covered by tests

anlambert edited the summary of this revision. (Show Details)Nov 16 2021, 2:46 PM

Build is green

Patch application report for D6641 (id=24150)

Rebasing onto 4eff12ad51...

First, rewinding head to replay your work on top of it...
Applying: browse/snapshot_context: Handle snapshots containing only releases

Changes applied before test

commit f4b352c1e51525584da3772de44502b4d06d189c
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Mon Nov 15 16:36:29 2021 +0100

    browse/snapshot_context: Handle snapshots containing only releases
    
    That specific case was not covered at all as such snapshots could not
    be found into the archive until recently.
    
    So ensure to handle such snapshots properly and add exhaustive tests
    to cover all possible execution paths.
    
    Related to T3724

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

Harbormaster completed remote builds in B25030: Diff 24150.Nov 16 2021, 3:01 PM

vlorentz added inline comments.Nov 16 2021, 3:05 PM

swh/web/browse/snapshot_context.py
586	are you sure it can't be a directory or content?

anlambert added inline comments.Nov 16 2021, 3:25 PM

swh/web/browse/snapshot_context.py
586	I did not see any HEAD alias pointing to other object types so far. I could raise an exception but does it really worth it ?

vlorentz added inline comments.Nov 16 2021, 3:55 PM

swh/web/browse/snapshot_context.py
586	Yeah, let's raise an exception, it will save us a few minutes if it ever shows up in Sentry

anlambert added inline comments.Nov 16 2021, 4:25 PM

swh/web/browse/snapshot_context.py
586	In fact, we are good here as branch aliases targeting object types different from revision or release are already filtered out by the process_snapshot_branches function. So we can keep that code as it is.