Page MenuHomeSoftware Heritage
Differential D5195

Use CoreSWHID instead of the deprecated SWHID class
ClosedPublic
Actions

Authored by vlorentz on Mar 4 2021, 2:31 PM.

Details

Reviewers
anlambert
Group Reviewers
Reviewers
Commits
rDFUSEf16f3a94748c: Use CoreSWHID instead of the deprecated SWHID class
Test Plan

Tests won't pass on Jenkins until swh-model > 1.0.0 is released.

Locally, tests pass only if swh-web-client is patched line this:

@@ -558,7 +566,7 @@ def content_raw(self, swhid: SWHIDish, **req_args) -> Iterator[bytes]:
         )
         r.raise_for_status()

-        yield from r.iter_content(chunk_size=None, decode_unicode=False)
+        yield r.content

     def origin_search(
         self,

Otherwise, r.iter_content blocks on the second iteration. I don't understand why.

EDIT: updating requests and requests-mock fixes the issue, somehow...

Diff Detail

Repository
rDFUSE FUSE virtual file system
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

vlorentz created this revision.Mar 4 2021, 2:31 PM
Herald added a reviewer: Reviewers. · View Herald TranscriptMar 4 2021, 2:31 PM
vlorentz edited the test plan for this revision. (Show Details)Mar 4 2021, 2:31 PM
swh-public-ci added a comment.Mar 4 2021, 2:34 PM

Build has FAILED

Patch application report for D5195 (id=18590)

Rebasing onto 57d375a3ff...

Current branch diff-target is up to date.
Changes applied before test
commit 616d94512c259a95ef960d536f54c08e0d6281b9
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Thu Mar 4 14:30:01 2021 +0100

    make all tests pass, assuming swh-web-client is patched
    
    line this:
@@ -558,7 +566,7 @@ def content_raw(self, swhid: SWHIDish, **req_args) -> Iterator[bytes]:
         )
         r.raise_for_status()

-        yield from r.iter_content(chunk_size=None, decode_unicode=False)
+        yield r.content

     def origin_search(
         self,
```

commit rDFUSEdb10ebd0354b
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date: Thu Mar 4 13:01:35 2021 +0100

some fixes

commit rDFUSE13f4c3db5ba6
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date: Wed Mar 3 16:13:54 2021 +0100

[wip] Use CoreSWHID instead of the deprecated SWHID class
Link to build: https://jenkins.softwareheritage.org/job/DFUSE/job/tests-on-diff/268/
See console output for more information: https://jenkins.softwareheritage.org/job/DFUSE/job/tests-on-diff/268/console
Harbormaster returned this revision to the author for changes because remote builds failed.Mar 4 2021, 2:34 PM
Harbormaster failed remote builds in B19682: Diff 18590!
vlorentz requested review of this revision.Mar 4 2021, 2:36 PM
vlorentz edited the test plan for this revision. (Show Details)
vlorentz retitled this revision from [wip] Use CoreSWHID instead of the deprecated SWHID class to [help wanted] Use CoreSWHID instead of the deprecated SWHID class.
vlorentz updated this revision to Diff 18591.Mar 4 2021, 2:38 PM
  • remove prints
swh-public-ci added a comment.Mar 4 2021, 2:41 PM

Build has FAILED

Patch application report for D5195 (id=18591)

Rebasing onto 57d375a3ff...

Current branch diff-target is up to date.
Changes applied before test
commit c71b172b896444bc42f13d2134b860160b469043
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Thu Mar 4 14:37:14 2021 +0100

    remove prints

commit 616d94512c259a95ef960d536f54c08e0d6281b9
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Thu Mar 4 14:30:01 2021 +0100

    make all tests pass, assuming swh-web-client is patched
    
    line this:
@@ -558,7 +566,7 @@ def content_raw(self, swhid: SWHIDish, **req_args) -> Iterator[bytes]:
         )
         r.raise_for_status()

-        yield from r.iter_content(chunk_size=None, decode_unicode=False)
+        yield r.content

     def origin_search(
         self,
```

commit rDFUSEdb10ebd0354b
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date: Thu Mar 4 13:01:35 2021 +0100

some fixes

commit rDFUSE13f4c3db5ba6
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date: Wed Mar 3 16:13:54 2021 +0100

[wip] Use CoreSWHID instead of the deprecated SWHID class
Link to build: https://jenkins.softwareheritage.org/job/DFUSE/job/tests-on-diff/269/
See console output for more information: https://jenkins.softwareheritage.org/job/DFUSE/job/tests-on-diff/269/console
Harbormaster failed remote builds in B19683: Diff 18591!Mar 4 2021, 2:42 PM
vlorentz edited the test plan for this revision. (Show Details)Mar 4 2021, 2:55 PM
vlorentz updated this revision to Diff 18593.Mar 4 2021, 2:57 PM
  • remove more prints
vlorentz retitled this revision from [help wanted] Use CoreSWHID instead of the deprecated SWHID class to Use CoreSWHID instead of the deprecated SWHID class.Mar 4 2021, 2:58 PM
swh-public-ci added a comment.Mar 4 2021, 3:01 PM

Build has FAILED

Patch application report for D5195 (id=18593)

Rebasing onto 57d375a3ff...

Current branch diff-target is up to date.
Changes applied before test
commit bdd4ad3a6fff49e0fc9c325477b3e7e6c00be8ce
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Thu Mar 4 14:37:14 2021 +0100

    remove prints

commit 616d94512c259a95ef960d536f54c08e0d6281b9
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Thu Mar 4 14:30:01 2021 +0100

    make all tests pass, assuming swh-web-client is patched
    
    line this:
@@ -558,7 +566,7 @@ def content_raw(self, swhid: SWHIDish, **req_args) -> Iterator[bytes]:
         )
         r.raise_for_status()

-        yield from r.iter_content(chunk_size=None, decode_unicode=False)
+        yield r.content

     def origin_search(
         self,
```

commit rDFUSEdb10ebd0354b
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date: Thu Mar 4 13:01:35 2021 +0100

some fixes

commit rDFUSE13f4c3db5ba6
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date: Wed Mar 3 16:13:54 2021 +0100

[wip] Use CoreSWHID instead of the deprecated SWHID class
Link to build: https://jenkins.softwareheritage.org/job/DFUSE/job/tests-on-diff/270/
See console output for more information: https://jenkins.softwareheritage.org/job/DFUSE/job/tests-on-diff/270/console
Harbormaster failed remote builds in B19685: Diff 18593!Mar 4 2021, 3:01 PM
swh-public-ci added a comment.Mar 4 2021, 3:30 PM

Build has FAILED

Patch application report for D5195 (id=18593)

Rebasing onto 57d375a3ff...

Current branch diff-target is up to date.
Changes applied before test
commit bdd4ad3a6fff49e0fc9c325477b3e7e6c00be8ce
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Thu Mar 4 14:37:14 2021 +0100

    remove prints

commit 616d94512c259a95ef960d536f54c08e0d6281b9
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Thu Mar 4 14:30:01 2021 +0100

    make all tests pass, assuming swh-web-client is patched
    
    line this:
@@ -558,7 +566,7 @@ def content_raw(self, swhid: SWHIDish, **req_args) -> Iterator[bytes]:
         )
         r.raise_for_status()

-        yield from r.iter_content(chunk_size=None, decode_unicode=False)
+        yield r.content

     def origin_search(
         self,
```

commit rDFUSEdb10ebd0354b
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date: Thu Mar 4 13:01:35 2021 +0100

some fixes

commit rDFUSE13f4c3db5ba6
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date: Wed Mar 3 16:13:54 2021 +0100

[wip] Use CoreSWHID instead of the deprecated SWHID class
Link to build: https://jenkins.softwareheritage.org/job/DFUSE/job/tests-on-diff/271/
See console output for more information: https://jenkins.softwareheritage.org/job/DFUSE/job/tests-on-diff/271/console
anlambert accepted this revision.Mar 4 2021, 3:56 PM
anlambert added a subscriber: anlambert.

Looks good to me.

swh/fuse/tests/test_cache.py
18

Why not using hash_to_hex here ?

Are swhid.object_id.hex() and hash_to_hex(swhid.object_id) equivalent ?
If it is, the first version is more pleasant to read.

This revision is now accepted and ready to land.Mar 4 2021, 3:56 PM
zack mentioned this in D5156: migrate from SWHID to new CoreSWHID class.Mar 4 2021, 4:57 PM
zack added a subscriber: zack.Mar 4 2021, 5:06 PM

LGTM too. I've also tried it locally, and it worked just fine, both interactively and when running the test suite.

I've only notice this (unrelated) warning:

WARNING:swh.core.cli:Could not load subcommand dataset: cannot import name 'swhid' from 'swh.model.identifiers' (/home/zack/dati/projects/sw-heritage/git/swh-environment/swh-model/swh/model/identifiers.py)
zack added inline comments.Mar 4 2021, 5:09 PM
swh/fuse/tests/test_cache.py
18

Could be my fault. I had used hex() in my branch, and @vlorentz might have gotten it from there.
I agree using hash_to_hex would more consistent with the rest of the code.

34

(and there's another hex() call here, probably my fault too :))

vlorentz updated this revision to Diff 18609.Mar 4 2021, 5:13 PM

apply comments + squash

swh-public-ci added a comment.Mar 4 2021, 5:18 PM

Build has FAILED

Patch application report for D5195 (id=18609)

Rebasing onto 57d375a3ff...

Current branch diff-target is up to date.
Changes applied before test
commit f16f3a94748c286c4fc1228096befb3fab41bec9
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Wed Mar 3 16:13:54 2021 +0100

    Use CoreSWHID instead of the deprecated SWHID class

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

Harbormaster failed remote builds in B19701: Diff 18609!Mar 4 2021, 5:18 PM
swh-public-ci added a comment.Mar 4 2021, 6:23 PM

Build has FAILED

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

swh-public-ci added a comment.Mar 4 2021, 6:28 PM

Build is green

Patch application report for D5195 (id=18609)

Rebasing onto 57d375a3ff...

Current branch diff-target is up to date.
Changes applied before test
commit f16f3a94748c286c4fc1228096befb3fab41bec9
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Wed Mar 3 16:13:54 2021 +0100

    Use CoreSWHID instead of the deprecated SWHID class

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

Harbormaster completed remote builds in B19701: Diff 18609.Mar 4 2021, 6:28 PM
Closed by commit rDFUSEf16f3a94748c: Use CoreSWHID instead of the deprecated SWHID class (authored by vlorentz). · Explain WhyMar 5 2021, 9:11 AM
This revision was automatically updated to reflect the committed changes.
vlorentz added a commit: rDFUSEf16f3a94748c: Use CoreSWHID instead of the deprecated SWHID class.

Revision Contents
Changeset List

PathSize
swh/
fuse/
36 lines
4 lines
fs/
59 lines
17 lines
14 lines
tests/
31 lines
data/
43 lines
14 lines
4 lines
7 lines

Diff 18616

View Options

swh/fuse/cache.py

Loading...
View Options

swh/fuse/cli.py

Loading...
View Options

swh/fuse/fs/artifact.py

Loading...
View Options

swh/fuse/fs/mountpoint.py

Loading...
View Options

swh/fuse/fuse.py

Loading...
View Options

swh/fuse/tests/api_url.py

Loading...
View Options

swh/fuse/tests/data/gen-api-data.py

Loading...
View Options

swh/fuse/tests/test_cache.py

Loading...
View Options

swh/fuse/tests/test_mountpoint.py

Loading...
View Options

swh/fuse/tests/test_revision.py

Loading...
About · Developer information · Legal