Page MenuHomeSoftware Heritage

Adapt storage.revision_get calls according to latest api change
ClosedPublic

Authored by ardumont on Thu, Sep 3, 5:48 PM.

Details

Summary

This also allows the new extra_headers conversion.
(current revisions might not have it yet but at some point, they will eventually get migrated to this [1] ;)

That may also fix the master build now (some new code got pushed in the repository since the new storage 0.14.0 release) [2]

[1] T2564

[2] https://jenkins.softwareheritage.org/job/DWAPPS/job/tests/1631/console

Related to T645

Test Plan

tox
(failing until release storage > 0.13.3)

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

ardumont created this revision.Thu, Sep 3, 5:48 PM

Build has FAILED

Patch application report for D3877 (id=13675)

Rebasing onto 9c41d9f90f...

Current branch diff-target is up to date.
Changes applied before test
commit 872745a95d3625dfb8a49f030b7a395b3e1abdb9
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Thu Sep 3 17:48:32 2020 +0200

    Adapt storage.revision_get calls according to latest api change
    
    This also allows the new extra_headers conversion.
    
    Related to T645

Link to build: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/353/
See console output for more information: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/353/console

ardumont edited the summary of this revision. (Show Details)Thu, Sep 3, 5:56 PM
ardumont added inline comments.Thu, Sep 3, 6:06 PM
swh/web/common/converters.py
265

I dropped this as I initially thought we could only use Revision here...
In the end, we cannot as some service function (the one calling revision_log) enrich the revision with a children key).
So I just allowed passing new revision.

Tell me if we wanna keep that previous docstring version.
(i'll probably revert that deletion tomorrow)

vlorentz added inline comments.
swh/web/common/converters.py
265

you could make a subclass of Revision with that extra attribute

ardumont added inline comments.Fri, Sep 4, 10:02 AM
swh/web/common/converters.py
265

yes, sounds like a good idea.

ardumont added inline comments.
swh/web/common/converters.py
265

@anlambert ^ What do you think?

If we agree, any particular suggestion name, and location for its declaration?

ardumont added inline comments.Fri, Sep 4, 1:30 PM
swh/web/common/converters.py
265

In the end, that makes lots of changes for just one case... so i won't.
(also i'd like to deploy this soon so the deployment is no longer blocked)

Build has FAILED

Patch application report for D3877 (id=13675)

Rebasing onto 9c41d9f90f...

Current branch diff-target is up to date.
Changes applied before test
commit 872745a95d3625dfb8a49f030b7a395b3e1abdb9
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Thu Sep 3 17:48:32 2020 +0200

    Adapt storage.revision_get calls according to latest api change
    
    This also allows the new extra_headers conversion.
    
    Related to T645

Link to build: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/354/
See console output for more information: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/354/console

anlambert accepted this revision.Fri, Sep 4, 2:03 PM

Looks good to me.

swh/web/tests/common/test_converters.py
265

you can remove that line, the id will be automatically computed by the model

This revision is now accepted and ready to land.Fri, Sep 4, 2:03 PM

Build has FAILED

Patch application report for D3877 (id=13675)

Rebasing onto 9c41d9f90f...

Current branch diff-target is up to date.
Changes applied before test
commit 872745a95d3625dfb8a49f030b7a395b3e1abdb9
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Thu Sep 3 17:48:32 2020 +0200

    Adapt storage.revision_get calls according to latest api change
    
    This also allows the new extra_headers conversion.
    
    Related to T645

Link to build: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/355/
See console output for more information: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/355/console

ardumont added inline comments.Fri, Sep 4, 2:12 PM
swh/web/tests/common/test_converters.py
265

yes, but heads up, then i'll have to change the id to check to "18d8be353ed3480476f032475e7c233eff7371d5" ;)

ardumont updated this revision to Diff 13704.Fri, Sep 4, 2:14 PM

Rebase and adapt slightly the revision model object

ardumont edited the summary of this revision. (Show Details)Fri, Sep 4, 2:17 PM

Build has FAILED

Patch application report for D3877 (id=13704)

Rebasing onto 5c1b8e1ad6...

Current branch diff-target is up to date.
Changes applied before test
commit 8545e0a44589ca18da3fd3c25d6073f2576b8baf
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Thu Sep 3 17:48:32 2020 +0200

    Adapt storage.revision_get calls according to latest api change
    
    This also allows the new extra_headers conversion.
    
    Related to T645

Link to build: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/356/
See console output for more information: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/356/console

ardumont edited the summary of this revision. (Show Details)Fri, Sep 4, 2:25 PM
ardumont edited the summary of this revision. (Show Details)Fri, Sep 4, 2:39 PM
ardumont edited the summary of this revision. (Show Details)

Build has FAILED

Patch application report for D3877 (id=13704)

Rebasing onto 5c1b8e1ad6...

Current branch diff-target is up to date.
Changes applied before test
commit 8545e0a44589ca18da3fd3c25d6073f2576b8baf
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Thu Sep 3 17:48:32 2020 +0200

    Adapt storage.revision_get calls according to latest api change
    
    This also allows the new extra_headers conversion.
    
    Related to T645

Link to build: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/357/
See console output for more information: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/357/console

build has failed [1]

yeah, well, i don't understand it yet and i don't know how to run it locally...

[1] https://forge.softwareheritage.org/harbormaster/unit/view/918517/

build has failed [1]

yeah, well, i don't understand it yet and i don't know how to run it locally...

[1] https://forge.softwareheritage.org/harbormaster/unit/view/918517/

I managed to track the error and it comes from the storage:

  File "/home/anlambert/swh/swh-environment/swh-web/swh/web/browse/views/revision.py", line 182, in _revision_diff
    changes = service.diff_revision(sha1_git)
  File "/home/anlambert/swh/swh-environment/swh-web/swh/web/common/service.py", line 1221, in diff_revision
    changes = diff.diff_revision(storage, rev_sha1_git_bin, track_renaming=True)
  File "/home/anlambert/swh/swh-environment/swh-storage/swh/storage/algos/diff.py", line 411, in diff_revision
    if rev_data["parents"]:
TypeError: 'Revision' object is not subscriptable

build has failed [1]

yeah, well, i don't understand it yet and i don't know how to run it locally...

[1] https://forge.softwareheritage.org/harbormaster/unit/view/918517/

I managed to track the error and it comes from the storage:

  File "/home/anlambert/swh/swh-environment/swh-web/swh/web/browse/views/revision.py", line 182, in _revision_diff
    changes = service.diff_revision(sha1_git)
  File "/home/anlambert/swh/swh-environment/swh-web/swh/web/common/service.py", line 1221, in diff_revision
    changes = diff.diff_revision(storage, rev_sha1_git_bin, track_renaming=True)
  File "/home/anlambert/swh/swh-environment/swh-storage/swh/storage/algos/diff.py", line 411, in diff_revision
    if rev_data["parents"]:
TypeError: 'Revision' object is not subscriptable

And here is the fix:

diff --git a/swh/storage/algos/diff.py b/swh/storage/algos/diff.py
index a22d47b9..459b9bd4 100644
--- a/swh/storage/algos/diff.py
+++ b/swh/storage/algos/diff.py
@@ -27,7 +27,7 @@ def _get_rev(storage, rev_id):
     """
     Return revision data from swh storage.
     """
-    return list(storage.revision_get([rev_id]))[0]
+    return storage.revision_get([rev_id])[0].to_dict()
 
 
 class _RevisionChangesList(object):

i don't know how to run it locally...

make test-frontend-ui helped to reproduce.

thanks, i was nowhere near that yet!

ardumont added a comment.EditedFri, Sep 4, 3:38 PM

And here is the fix:
...

Thanks, iterated over it D3883.
I'll release a 0.14.1 with it.

Build is green

Patch application report for D3877 (id=13704)

Rebasing onto 5c1b8e1ad6...

Current branch diff-target is up to date.
Changes applied before test
commit 8545e0a44589ca18da3fd3c25d6073f2576b8baf
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Thu Sep 3 17:48:32 2020 +0200

    Adapt storage.revision_get calls according to latest api change
    
    This also allows the new extra_headers conversion.
    
    Related to T645

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

Build is green

finally \m/