Page MenuHomeSoftware Heritage

Add typing to detect_metadata() and related functions
ClosedPublic

Authored by vlorentz on Jun 29 2022, 11:02 AM.

Diff Detail

Repository
rDCIDX Metadata indexer
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

Build is green

Patch application report for D8045 (id=28984)

Rebasing onto 1be4e184d4...

Current branch diff-target is up to date.
Changes applied before test
commit eef33fc72d1ec86db15c12ca2eafad03719e0e48
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Wed Jun 29 11:01:35 2022 +0200

    Add typing to detect_metadata() and related functions

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

Build is green

Patch application report for D8045 (id=28987)

Rebasing onto 1be4e184d4...

Current branch diff-target is up to date.
Changes applied before test
commit f7a4bf4e04b3ac4c2fa89cf9b8a5c22e5f0c4d12
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Wed Jun 29 11:01:35 2022 +0200

    Add typing to detect_metadata() and related functions

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

douardda added inline comments.
swh/indexer/metadata.py
205

I don't understand this assertion. Is it that the data argument is now deprecated?

Also looks to me that with this chunk, this diff is not "just" adding typing.

218

same, is this detected_files removal just a typing kinda stuff? looks not obvious to me

282–284

ah I see, you moved the behavior here... Some explanations (in the commit message at least) would help following what's going on.

olasd added a subscriber: olasd.

Thanks.

I'm slightly surprised by the change of signature for translate_directory_intrinsic_metadata without change to the docstring. AFAICT this function now does the detection itself, so maybe it deserves an update.

This revision is now accepted and ready to land.Jul 4 2022, 11:49 AM
In D8045#209752, @olasd wrote:

AFAICT this function now does the detection itself, so maybe it deserves an update.

Ah, good point

swh/indexer/metadata.py
205

it removes code that was outright broken, which was detected by mypy.

282–284

heh... it was needed in order for type annotations to make sense, because the code was badly organized.

I'll mention it in the commit message

swh/indexer/metadata.py
205

by "outright broken", I mean that when leaving the removed branch, dir_ was a dict containing an entries key, instead of being the list itself, as expected by the rest of the code (entry["type"] for entry in dir_)

update docstring and make commit msg more descriptive

swh/indexer/metadata.py
205

shouldn't the docstring be updated then at least?

282–284

heh... it was needed in order for type annotations to make sense, because the code was badly organized.

I'll mention it in the commit message

thx

Build is green

Patch application report for D8045 (id=29113)

Rebasing onto 1be4e184d4...

Current branch diff-target is up to date.
Changes applied before test
commit dc035a669329272713b0b977ba2f685501e80ba0
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Wed Jun 29 11:01:35 2022 +0200

    Add typing to detect_metadata() and related functions
    
    This also moves the call to `detect_metadata()` to
    `translate_directory_intrinsic_metadata` so type annotations make more
    sense; and remove a dead/broken code branch in
    `DirectoryMetadataIndexer.index()` that was detected by mypy.

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

Build is green

Patch application report for D8045 (id=29116)

Rebasing onto 1be4e184d4...

Current branch diff-target is up to date.
Changes applied before test
commit 8c5a84459d4436af7b4b751e48091a99bce0a7d1
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Wed Jun 29 11:01:35 2022 +0200

    Add typing to detect_metadata() and related functions
    
    This also moves the call to `detect_metadata()` to
    `translate_directory_intrinsic_metadata` so type annotations make more
    sense; and remove a dead/broken code branch in
    `DirectoryMetadataIndexer.index()` that was detected by mypy.

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