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

  • This file was added.
import io
import traceback
from multiprocessing import Process, Queue
from typing import Dict, NewType
# The internal Mercurial API is not guaranteed to be stable.
import mercurial.ui # type: ignore
from mercurial import context, hg
NULLID = mercurial.node.nullid
HgNodeId = NewType("HgNodeId", bytes)
Repository = hg.localrepo
BaseContext = context.basectx
def repository(path: str) -> hg.localrepo:
ui = mercurial.ui.ui.load()
return hg.repository(ui, path.encode())
def branches(repo: hg.localrepo) -> Dict[bytes, HgNodeId]:
marmouteUnsubmitted
Done
Inline Actions

Same feedback about documenting the what data we are looking for.

marmoute: Same feedback about documenting the what data we are looking for.
"""List repository named branches and their tip node."""
result = {}
for tag, heads, tip, isclosed in repo.branchmap().iterbranches():
if isclosed:
continue
result[tag] = tip
return result
class CloneTimeout(Exception):
marmouteUnsubmitted
Done
Inline Actions

This seems duplicated with the previous file ? Why is their two of them ?

marmoute: This seems duplicated with the previous file ? Why is their two of them ?
marmouteUnsubmitted
Done
Inline Actions

Can you give some details about what happened on this topic ?

marmoute: Can you give some details about what happened on this topic ?
acezarAuthorUnsubmitted
Done
Inline Actions

removed the duplication. kept the hgutil.py version

acezar: removed the duplication. kept the `hgutil.py` version
marmouteUnsubmitted
Done
Inline Actions

Okay thanks

marmoute: Okay thanks
pass
class CloneFailure(Exception):
pass
def _clone_task(src: str, dest: str, errors: Queue) -> None:
"""Clone task to run in a subprocess.
Args:
marmouteUnsubmitted
Done
Inline Actions

please document this.

marmoute: please document this.
src: clone source
dest: clone destination
errors: message queue to communicate errors
"""
try:
hg.clone(mercurial.ui.ui.load(), {}, src.encode(), dest.encode())
except Exception as e:
exc_buffer = io.StringIO()
traceback.print_exc(file=exc_buffer)
errors.put_nowait(exc_buffer.getvalue())
marmouteUnsubmitted
Done
Inline Actions

please document this.

marmoute: please document this.
raise e
def clone(src: str, dest: str, timeout: int) -> None:
"""Clone a repository with timeout.
Args:
src: clone source
dest: clone destination
timeout: timeout in seconds
"""
errors: Queue = Queue()
process = Process(target=_clone_task, args=(src, dest, errors))
process.start()
process.join(timeout)
if process.is_alive():
process.terminate()
process.join(1)
if process.is_alive():
process.kill()
raise CloneTimeout(src, timeout)
if not errors.empty():
raise CloneFailure(src, dest, errors.get())
View Options

swh/fuse/cli.py

  • This file was added.
import io
import traceback
from multiprocessing import Process, Queue
from typing import Dict, NewType
# The internal Mercurial API is not guaranteed to be stable.
import mercurial.ui # type: ignore
from mercurial import context, hg
NULLID = mercurial.node.nullid
HgNodeId = NewType("HgNodeId", bytes)
Repository = hg.localrepo
BaseContext = context.basectx
def repository(path: str) -> hg.localrepo:
ui = mercurial.ui.ui.load()
return hg.repository(ui, path.encode())
def branches(repo: hg.localrepo) -> Dict[bytes, HgNodeId]:
marmouteUnsubmitted
Done
Inline Actions

Same feedback about documenting the what data we are looking for.

marmoute: Same feedback about documenting the what data we are looking for.
"""List repository named branches and their tip node."""
result = {}
for tag, heads, tip, isclosed in repo.branchmap().iterbranches():
if isclosed:
continue
result[tag] = tip
return result
class CloneTimeout(Exception):
marmouteUnsubmitted
Done
Inline Actions

This seems duplicated with the previous file ? Why is their two of them ?

marmoute: This seems duplicated with the previous file ? Why is their two of them ?
marmouteUnsubmitted
Done
Inline Actions

Can you give some details about what happened on this topic ?

marmoute: Can you give some details about what happened on this topic ?
acezarAuthorUnsubmitted
Done
Inline Actions

removed the duplication. kept the hgutil.py version

acezar: removed the duplication. kept the `hgutil.py` version
marmouteUnsubmitted
Done
Inline Actions

Okay thanks

marmoute: Okay thanks
pass
class CloneFailure(Exception):
pass
def _clone_task(src: str, dest: str, errors: Queue) -> None:
"""Clone task to run in a subprocess.
Args:
marmouteUnsubmitted
Done
Inline Actions

please document this.

marmoute: please document this.
src: clone source
dest: clone destination
errors: message queue to communicate errors
"""
try:
hg.clone(mercurial.ui.ui.load(), {}, src.encode(), dest.encode())
except Exception as e:
exc_buffer = io.StringIO()
traceback.print_exc(file=exc_buffer)
errors.put_nowait(exc_buffer.getvalue())
marmouteUnsubmitted
Done
Inline Actions

please document this.

marmoute: please document this.
raise e
def clone(src: str, dest: str, timeout: int) -> None:
"""Clone a repository with timeout.
Args:
src: clone source
dest: clone destination
timeout: timeout in seconds
"""
errors: Queue = Queue()
process = Process(target=_clone_task, args=(src, dest, errors))
process.start()
process.join(timeout)
if process.is_alive():
process.terminate()
process.join(1)
if process.is_alive():
process.kill()
raise CloneTimeout(src, timeout)
if not errors.empty():
raise CloneFailure(src, dest, errors.get())
View Options

swh/fuse/fs/artifact.py

  • This file was added.
import io
import traceback
from multiprocessing import Process, Queue
from typing import Dict, NewType
# The internal Mercurial API is not guaranteed to be stable.
import mercurial.ui # type: ignore
from mercurial import context, hg
NULLID = mercurial.node.nullid
HgNodeId = NewType("HgNodeId", bytes)
Repository = hg.localrepo
BaseContext = context.basectx
def repository(path: str) -> hg.localrepo:
ui = mercurial.ui.ui.load()
return hg.repository(ui, path.encode())
def branches(repo: hg.localrepo) -> Dict[bytes, HgNodeId]:
marmouteUnsubmitted
Done
Inline Actions

Same feedback about documenting the what data we are looking for.

marmoute: Same feedback about documenting the what data we are looking for.
"""List repository named branches and their tip node."""
result = {}
for tag, heads, tip, isclosed in repo.branchmap().iterbranches():
if isclosed:
continue
result[tag] = tip
return result
class CloneTimeout(Exception):
marmouteUnsubmitted
Done
Inline Actions

This seems duplicated with the previous file ? Why is their two of them ?

marmoute: This seems duplicated with the previous file ? Why is their two of them ?
marmouteUnsubmitted
Done
Inline Actions

Can you give some details about what happened on this topic ?

marmoute: Can you give some details about what happened on this topic ?
acezarAuthorUnsubmitted
Done
Inline Actions

removed the duplication. kept the hgutil.py version

acezar: removed the duplication. kept the `hgutil.py` version
marmouteUnsubmitted
Done
Inline Actions

Okay thanks

marmoute: Okay thanks
pass
class CloneFailure(Exception):
pass
def _clone_task(src: str, dest: str, errors: Queue) -> None:
"""Clone task to run in a subprocess.
Args:
marmouteUnsubmitted
Done
Inline Actions

please document this.

marmoute: please document this.
src: clone source
dest: clone destination
errors: message queue to communicate errors
"""
try:
hg.clone(mercurial.ui.ui.load(), {}, src.encode(), dest.encode())
except Exception as e:
exc_buffer = io.StringIO()
traceback.print_exc(file=exc_buffer)
errors.put_nowait(exc_buffer.getvalue())
marmouteUnsubmitted
Done
Inline Actions

please document this.

marmoute: please document this.
raise e
def clone(src: str, dest: str, timeout: int) -> None:
"""Clone a repository with timeout.
Args:
src: clone source
dest: clone destination
timeout: timeout in seconds
"""
errors: Queue = Queue()
process = Process(target=_clone_task, args=(src, dest, errors))
process.start()
process.join(timeout)
if process.is_alive():
process.terminate()
process.join(1)
if process.is_alive():
process.kill()
raise CloneTimeout(src, timeout)
if not errors.empty():
raise CloneFailure(src, dest, errors.get())
View Options

swh/fuse/fs/mountpoint.py

  • This file was added.
import io
import traceback
from multiprocessing import Process, Queue
from typing import Dict, NewType
# The internal Mercurial API is not guaranteed to be stable.
import mercurial.ui # type: ignore
from mercurial import context, hg
NULLID = mercurial.node.nullid
HgNodeId = NewType("HgNodeId", bytes)
Repository = hg.localrepo
BaseContext = context.basectx
def repository(path: str) -> hg.localrepo:
ui = mercurial.ui.ui.load()
return hg.repository(ui, path.encode())
def branches(repo: hg.localrepo) -> Dict[bytes, HgNodeId]:
marmouteUnsubmitted
Done
Inline Actions

Same feedback about documenting the what data we are looking for.

marmoute: Same feedback about documenting the what data we are looking for.
"""List repository named branches and their tip node."""
result = {}
for tag, heads, tip, isclosed in repo.branchmap().iterbranches():
if isclosed:
continue
result[tag] = tip
return result
class CloneTimeout(Exception):
marmouteUnsubmitted
Done
Inline Actions

This seems duplicated with the previous file ? Why is their two of them ?

marmoute: This seems duplicated with the previous file ? Why is their two of them ?
marmouteUnsubmitted
Done
Inline Actions

Can you give some details about what happened on this topic ?

marmoute: Can you give some details about what happened on this topic ?
acezarAuthorUnsubmitted
Done
Inline Actions

removed the duplication. kept the hgutil.py version

acezar: removed the duplication. kept the `hgutil.py` version
marmouteUnsubmitted
Done
Inline Actions

Okay thanks

marmoute: Okay thanks
pass
class CloneFailure(Exception):
pass
def _clone_task(src: str, dest: str, errors: Queue) -> None:
"""Clone task to run in a subprocess.
Args:
marmouteUnsubmitted
Done
Inline Actions

please document this.

marmoute: please document this.
src: clone source
dest: clone destination
errors: message queue to communicate errors
"""
try:
hg.clone(mercurial.ui.ui.load(), {}, src.encode(), dest.encode())
except Exception as e:
exc_buffer = io.StringIO()
traceback.print_exc(file=exc_buffer)
errors.put_nowait(exc_buffer.getvalue())
marmouteUnsubmitted
Done
Inline Actions

please document this.

marmoute: please document this.
raise e
def clone(src: str, dest: str, timeout: int) -> None:
"""Clone a repository with timeout.
Args:
src: clone source
dest: clone destination
timeout: timeout in seconds
"""
errors: Queue = Queue()
process = Process(target=_clone_task, args=(src, dest, errors))
process.start()
process.join(timeout)
if process.is_alive():
process.terminate()
process.join(1)
if process.is_alive():
process.kill()
raise CloneTimeout(src, timeout)
if not errors.empty():
raise CloneFailure(src, dest, errors.get())
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