The current conversions done were a bit ambiguous, specifying the types clarifies the
need.
Details
- Reviewers
vlorentz - Group Reviewers
Reviewers - Commits
- rDLDGc662603fb825: Clarify local/remote heads type as those are hexadecimal bytes str
tox
Diff Detail
- Repository
- rDLDG Git loader
- Branch
- master
- Lint
Lint Skipped - Unit
Unit Tests Skipped - Build Status
Buildable 24004 Build 37445: Phabricator diff pipeline on jenkins Jenkins console · Jenkins Build 37444: arc lint + arc unit
Event Timeline
Build is green
Patch application report for D6348 (id=23067)
Rebasing onto 05d6d762d0...
Current branch diff-target is up to date.
Changes applied before test
commit 25cd52608ea214113b24bd3150f92e04766cb4de Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org> Date: Sun Sep 26 16:40:17 2021 +0200 Use correct logging instruction, let log.info format entries commit ffeff53604b4d03b760bc6c3442707fb20f49a99 Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org> Date: Sun Sep 26 16:22:16 2021 +0200 Drop unnecessary bytes/hex conversion branch.target is already bytes As per swh.model.model.Snapshot definition
See https://jenkins.softwareheritage.org/job/DLDG/job/tests-on-diff/115/ for more details.
swh/loader/git/loader.py | ||
---|---|---|
81 | That's not the same behavior. hashutil.hash_to_hex(branch.target).encode() is equivalent to hashutil.hash_to_bytehex(branch.target); ie. the hexadecimal representation of the hash, but in bytes. The new code writes the hash digest directly. This should be caught by tests, though... |
swh/loader/git/loader.py | ||
---|---|---|
81 | so true... (both) |
swh/loader/git/loader.py | ||
---|---|---|
81 | I'll explicit the types instead then. |
- Clarify local/remote heads type as those are hexadecimal bytes str
- Use correct logging instruction, let log.info format entries
Build is green
Patch application report for D6348 (id=23071)
Rebasing onto 05d6d762d0...
Current branch diff-target is up to date.
Changes applied before test
commit b09437818e0eb45550df9bbb2d7914d64827539a Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org> Date: Sun Sep 26 16:40:17 2021 +0200 Use correct logging instruction, let log.info format entries commit bd81a94b2a06fa8afa3a53012383ca17bbc5ede0 Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org> Date: Sun Sep 26 16:22:16 2021 +0200 Clarify local/remote heads type as those are hexadecimal bytes str It was not clear what all those conversion were about. Hopefully, identifying more clearly the types help now.
See https://jenkins.softwareheritage.org/job/DLDG/job/tests-on-diff/116/ for more details.
swh/loader/git/loader.py | ||
---|---|---|
41 | why not a NewType? |
swh/loader/git/loader.py | ||
---|---|---|
41 | because, you know, that ^ i knew how to do immediately. Also, before i dedicate more time on this, are you fine with the idea of this type? |
swh/loader/git/loader.py | ||
---|---|---|
41 | Maybe add a comment/docstring to document what this type is/represent? |
swh/loader/git/loader.py | ||
---|---|---|
41 | Thanks updated with your suggestions to both ;) |
Build is green
Patch application report for D6348 (id=23081)
Rebasing onto 05d6d762d0...
Current branch diff-target is up to date.
Changes applied before test
commit 47d57f12fa674707a2d11def82e5b9a4e17d1cf3 Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org> Date: Sun Sep 26 16:22:16 2021 +0200 Clarify local/remote heads type as those are hexadecimal bytes str The current conversions done were a bit ambiguous, specifying the types clarifies the need.
See https://jenkins.softwareheritage.org/job/DLDG/job/tests-on-diff/118/ for more details.
Build is green
Patch application report for D6348 (id=23082)
Could not rebase; Attempt merge onto f4556e0113...
Updating f4556e0..7b873a0 Fast-forward swh/loader/package/opam/loader.py | 181 ++++++++++++++++------------- swh/loader/package/opam/tests/test_opam.py | 50 +++++++- 2 files changed, 146 insertions(+), 85 deletions(-)
Changes applied before test
commit 7b873a02d6718456951a8330d402bd31a4913890 Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org> Date: Fri Sep 24 13:37:40 2021 +0200 opam: Define a production mode (default) When running in production, the workers should expect the opam root directory to be present (externally maintained). When running in standalone (non maintenance), the loader should create the opam root folder if not present so it can work out of the box. Docker workers should have that `production` key set to false so they can run in standalone mode. Related to T3590 commit b2d175965abed6d116a9f70e484ba24ea01f63cc Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org> Date: Tue Sep 21 20:16:53 2021 +0200 Allow opam loader to actually use multi-instance opam root It allow the opam loader to reuse existing opam root with multiple instances. It's the complementary code that goes with the loader adaptation [1]. As the `opam show` (cli) [2] version currently packaged does not support the means to enclose the metadata extraction per opam instance (when sharing the same opam root), we actually work around this by opening internal details to opam. [1] D6316 [2] `opam show` is currently the interface we are using to extract and list information about a package. It does work on standalone opam root folder but it comes short when sharing multiple instances within one opam root (for now).
See https://jenkins.softwareheritage.org/job/DLDBASE/job/tests-on-diff/572/ for more details.
Build is green
Patch application report for D6348 (id=23083)
Rebasing onto 05d6d762d0...
Current branch diff-target is up to date.
Changes applied before test
commit 47d57f12fa674707a2d11def82e5b9a4e17d1cf3 Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org> Date: Sun Sep 26 16:22:16 2021 +0200 Clarify local/remote heads type as those are hexadecimal bytes str The current conversions done were a bit ambiguous, specifying the types clarifies the need.
See https://jenkins.softwareheritage.org/job/DLDG/job/tests-on-diff/119/ for more details.
Build has FAILED
Patch application report for D6348 (id=23141)
Rebasing onto d3976ca650...
Current branch diff-target is up to date.
Changes applied before test
commit 49f74b764c64b3c1617a1fc5b3935aef262e9a08 Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org> Date: Sun Sep 26 16:22:16 2021 +0200 Clarify local/remote heads type as those are hexadecimal bytes str The current conversions done were a bit ambiguous, specifying the types clarifies the need.
Link to build: https://jenkins.softwareheritage.org/job/DLDG/job/tests-on-diff/122/
See console output for more information: https://jenkins.softwareheritage.org/job/DLDG/job/tests-on-diff/122/console
Fix type
swh/loader/git/loader.py | ||
---|---|---|
267–268 | because swh/loader/git/loader.py:287: error: Argument 1 to "filter_refs" has incompatible type "Dict[bytes, HexBytes]"; expected "Dict[bytes, bytes]" swh/loader/git/loader.py:287: note: "Dict" is invariant -- see https://mypy.readthedocs.io/en/stable/common_issues.html#variance swh/loader/git/loader.py:287: note: Consider using "Mapping" instead, which is covariant in the value type Found 1 error in 1 file (checked 16 source files) and *sigh* |
Build is green
Patch application report for D6348 (id=23144)
Rebasing onto d3976ca650...
Current branch diff-target is up to date.
Changes applied before test
commit c662603fb8251eda3e3beec8a425b6c21968df1e Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org> Date: Sun Sep 26 16:22:16 2021 +0200 Clarify local/remote heads type as those are hexadecimal bytes str The current conversions done were a bit ambiguous, specifying the types clarifies the need.
See https://jenkins.softwareheritage.org/job/DLDG/job/tests-on-diff/123/ for more details.