Page MenuHomeSoftware Heritage
Differential D3877

Adapt storage.revision_get calls according to latest api change
ClosedPublic
Actions

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

Details

Reviewers
anlambert
Group Reviewers
Reviewers
Maniphest Tasks
T645: Type swh-storage endpoints with swh.model objects
Commits
rDWAPPS8545e0a44589: Adapt storage.revision_get calls according to latest api change
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.Sep 3 2020, 5:48 PM
Herald added a reviewer: Reviewers. · View Herald TranscriptSep 3 2020, 5:48 PM
ardumont mentioned this in D3863: Refactor revision_get storage API to return Revision objects.Sep 3 2020, 5:49 PM
swh-public-ci added a comment.Sep 3 2020, 5:52 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

Harbormaster failed remote builds in B14906: Diff 13675!Sep 3 2020, 5:52 PM
ardumont edited the summary of this revision. (Show Details)Sep 3 2020, 5:56 PM
ardumont added inline comments.Sep 3 2020, 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 a subscriber: vlorentz.Sep 3 2020, 8:39 PM
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.Sep 4 2020, 10:02 AM
swh/web/common/converters.py
265

yes, sounds like a good idea.

ardumont added a subscriber: anlambert.Sep 4 2020, 11:05 AM
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.Sep 4 2020, 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)

swh-public-ci added a comment.Sep 4 2020, 1:37 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/354/
See console output for more information: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/354/console

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

Looks good to me.

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

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

This revision is now accepted and ready to land.Sep 4 2020, 2:03 PM
swh-public-ci added a comment.Sep 4 2020, 2:07 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.Sep 4 2020, 2:12 PM
swh/web/tests/common/test_converters.py
264

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

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

Rebase and adapt slightly the revision model object

ardumont edited the summary of this revision. (Show Details)Sep 4 2020, 2:17 PM
swh-public-ci added a comment.Sep 4 2020, 2:20 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

Harbormaster failed remote builds in B14931: Diff 13704!Sep 4 2020, 2:20 PM
ardumont edited the summary of this revision. (Show Details)Sep 4 2020, 2:25 PM
ardumont edited the summary of this revision. (Show Details)Sep 4 2020, 2:39 PM
ardumont edited the summary of this revision. (Show Details)
swh-public-ci added a comment.Sep 4 2020, 2:45 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/357/
See console output for more information: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/357/console

ardumont added a comment.Sep 4 2020, 2:56 PM

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/

anlambert added a comment.Sep 4 2020, 2:57 PM
In D3877#95930, @ardumont wrote:

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
anlambert added a comment.Sep 4 2020, 3:10 PM
In D3877#95931, @anlambert wrote:
In D3877#95930, @ardumont wrote:

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):
ardumont added a comment.Sep 4 2020, 3:13 PM

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.EditedSep 4 2020, 3:38 PM

And here is the fix:
...

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

swh-public-ci added a comment.Sep 4 2020, 3:59 PM

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.

Harbormaster completed remote builds in B14931: Diff 13704.Sep 4 2020, 3:59 PM
ardumont added a comment.Sep 4 2020, 4:00 PM

Build is green

finally \m/

Closed by commit rDWAPPS8545e0a44589: Adapt storage.revision_get calls according to latest api change (authored by ardumont). · Explain WhySep 4 2020, 4:00 PM
This revision was automatically updated to reflect the committed changes.
ardumont added a commit: rDWAPPS8545e0a44589: Adapt storage.revision_get calls according to latest api change.

Revision Contents
Changeset List

PathSize
swh/
web/
common/
51 lines
81 lines
tests/
common/
118 lines
6 lines
4 lines
2 lines

Diff 13707

View Options

swh/web/common/converters.py

Loading...
View Options

swh/web/common/service.py

Loading...
View Options

swh/web/tests/common/test_converters.py

Loading...
View Options

swh/web/tests/conftest.py

Loading...
View Options

swh/web/tests/data.py

Loading...
View Options

swh/web/tests/strategies.py

Loading...
About · Developer information · Legal