Page MenuHomeSoftware Heritage
Differential D3196

package.loader: Check for snapshot id prior to reifying model object
ClosedPublic
Actions

Authored by ardumont on May 29 2020, 12:24 PM.

Details

Reviewers
olasd
Group Reviewers
Reviewers
Maniphest Tasks
T2428: Unstuck failing loaders in staging
Commits
rDLDBASE385843d863ee: package.loader: Check for snapshot id prior to reifying model object
Summary

This is an issue mostly encountered on staging. But technically, the error
could happen in production as snapshot_get_all_branches could return None as
well (which is currently ignored). If that happens though, Snapshot.from_dict
raises.

Most proeminent in staging, because we can mount partial dump from production.
Thus not everything exists. Typically, recently we mounted a partial dump of
origins/origin-visits only. This means origin visits will target inexisting
snapshots [1]

Thix fix ensures we can use staging nonetheless. This also improves the current
implementation path so win.

Related to T2428

[1] https://sentry.softwareheritage.org/share/issue/4eb69ea2cc314c4ab811809ff60c358f/

Test Plan

tox

Diff Detail

Event Timeline

ardumont created this revision.May 29 2020, 12:24 PM
Herald added a reviewer: Reviewers. · View Herald TranscriptMay 29 2020, 12:24 PM
swh-public-ci added a comment.May 29 2020, 12:25 PM

Build is green

Patch application report for D3196 (id=11351)

Rebasing onto c6be4c0fd4...

Current branch diff-target is up to date.
Changes applied before test
commit 162478a5954a9fc77f1cedf1fb947e97a97557c2
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Fri May 29 12:14:14 2020 +0200

    package.loader: Check for snapshot id prior to reify the model object
    
    This is an issue encountered on staging alone for now. As we can mount partial
    dump from production, not everything exists. Typically dangling snapshot from
    origin visits currently. This ensures we can use staging nonetheless.
    
    Related to T2428

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

Harbormaster completed remote builds in B12547: Diff 11351.May 29 2020, 12:25 PM
ardumont edited the summary of this revision. (Show Details)May 29 2020, 12:26 PM
ardumont added projects: Core Loader, Staging environment.May 29 2020, 12:28 PM
ardumont removed a project: Staging environment.
ardumont edited the summary of this revision. (Show Details)May 29 2020, 12:31 PM
ardumont edited the summary of this revision. (Show Details)May 29 2020, 12:44 PM
olasd accepted this revision.May 29 2020, 12:52 PM
olasd added a subscriber: olasd.

Thanks!

Please rename the snap variable to something meaningful :)

swh/loader/package/loader.py
127–128

snap -> snapshot_dict

This revision is now accepted and ready to land.May 29 2020, 12:52 PM
ardumont updated this revision to Diff 11352.May 29 2020, 1:21 PM
ardumont edited the summary of this revision. (Show Details)
  • Adapt according to review
  • Align git commit with diff description
ardumont edited the summary of this revision. (Show Details)May 29 2020, 1:22 PM
ardumont updated this revision to Diff 11353.May 29 2020, 1:22 PM
ardumont edited the summary of this revision. (Show Details)

Rework commit message

swh-public-ci added a comment.May 29 2020, 1:23 PM

Build is green

Patch application report for D3196 (id=11352)

Rebasing onto faa138226d...

First, rewinding head to replay your work on top of it...
Applying: package.loader: Check for snapshot id prior to reifying model object
Changes applied before test
commit da6795a2b178bd139ec2ac012ce11b1116816758
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Fri May 29 12:14:14 2020 +0200

    package.loader: Check for snapshot id prior to reifying model object
    
    This is an issue mostly encountered on staging. But technically, the error
    could happen in production as snapshot_get_all_branches could return None as
    well (which is currently ignored). If that happens though, Snapshot.from_dict
    raises.
    
    Most proeminent in staging, because we can mount partial dump from production.
    Thus not everything exists. Typically, recently we mounted a partial dump of
    origins/origin-visits only. This means origin visits will target not
    inexisting [1]
    
    Thix fix ensures we can use staging nonetheless. This also improves the current
    implementation path so win.
    
    Related to T2428
    
    [1] https://sentry.softwareheritage.org/share/issue/4eb69ea2cc314c4ab811809ff60c358f/

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

Harbormaster completed remote builds in B12549: Diff 11352.May 29 2020, 1:23 PM
ardumont retitled this revision from package.loader: Check for snapshot id prior to reify the model object to package.loader: Check for snapshot id prior to reifying model object.May 29 2020, 1:23 PM
swh-public-ci added a comment.May 29 2020, 1:24 PM

Build is green

Patch application report for D3196 (id=11353)

Rebasing onto faa138226d...

First, rewinding head to replay your work on top of it...
Applying: package.loader: Check for snapshot id prior to reifying model object
Changes applied before test
commit 93a748ca2884cb7dd6bda2175b2281205437cffa
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Fri May 29 12:14:14 2020 +0200

    package.loader: Check for snapshot id prior to reifying model object
    
    This is an issue mostly encountered on staging. But technically, the error
    could happen in production as snapshot_get_all_branches could return None as
    well (which is currently ignored). If that happens though, `Snapshot.from_dict`
    raises.
    
    Most proeminent in staging, because we can mount partial dump from production.
    Thus not everything exists. Typically, recently we mounted a partial dump of
    origins/origin-visits only. This means origin visits will target inexisting
    snapshots [1]
    
    Thix fix ensures we can use staging nonetheless. This also improves the current
    implementation path so win.
    
    Related to T2428
    
    [1] https://sentry.softwareheritage.org/share/issue/4eb69ea2cc314c4ab811809ff60c358f/

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

Harbormaster completed remote builds in B12550: Diff 11353.May 29 2020, 1:24 PM
ardumont updated this revision to Diff 11366.May 29 2020, 3:57 PM

Rebase on latest master

swh-public-ci added a comment.May 29 2020, 3:58 PM

Build is green

Patch application report for D3196 (id=11366)

Rebasing onto dd2dc47667...

Current branch diff-target is up to date.
Changes applied before test
commit 385843d863ee38a59cbdc7c87d470d8d9a4d5db1
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Fri May 29 12:14:14 2020 +0200

    package.loader: Check for snapshot id prior to reifying model object
    
    This is an issue mostly encountered on staging. But technically, the error
    could happen in production as snapshot_get_all_branches could return None as
    well (which is currently ignored). If that happens though, `Snapshot.from_dict`
    raises.
    
    Most proeminent in staging, because we can mount partial dump from production.
    Thus not everything exists. Typically, recently we mounted a partial dump of
    origins/origin-visits only. This means origin visits will target inexisting
    snapshots [1]
    
    Thix fix ensures we can use staging nonetheless. This also improves the current
    implementation path so win.
    
    Related to T2428
    
    [1] https://sentry.softwareheritage.org/share/issue/4eb69ea2cc314c4ab811809ff60c358f/

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

Harbormaster completed remote builds in B12564: Diff 11366.May 29 2020, 3:58 PM
Closed by commit rDLDBASE385843d863ee: package.loader: Check for snapshot id prior to reifying model object (authored by ardumont). · Explain WhyMay 29 2020, 3:59 PM
This revision was automatically updated to reflect the committed changes.
ardumont added a commit: rDLDBASE385843d863ee: package.loader: Check for snapshot id prior to reifying model object.

Revision Contents
Changeset List

PathSizeCoverage (All)Coverage (Touched)
swh/
loader/
package/
9 lines93%
Loading...

Diff 11351

View Options

swh/loader/package/loader.py

Loading...
About · Developer information · Legal