Page MenuHomeSoftware Heritage

browse: Add typing to view function signatures
ClosedPublic

Authored by anlambert on Jun 8 2022, 3:55 PM.

Details

Summary

Because parameters of view functions for browse web application were
not typed, mypy was not processing the body of those functions and
thus typing errors could be missed.

So add typing to these function signatures and fix new mypy errors now
new code is processed.

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

Build has FAILED

Patch application report for D7973 (id=28725)

Rebasing onto 72b6022e09...

Current branch diff-target is up to date.
Changes applied before test
commit 918218c88a8d13fd30823f3d3070fd664c06df93
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Wed Jun 8 14:47:52 2022 +0200

    browse: Add typing to view function signatures
    
    Because parameters of view functions for browse web application were
    not typed, mypy was not processing the body of those functions and
    thus typing errors could be missed.
    
    So add typing to these function signatures and fix new mypy errors now
    new code is processed.

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

Harbormaster returned this revision to the author for changes because remote builds failed.Jun 8 2022, 4:20 PM
Harbormaster failed remote builds in B29806: Diff 28725!

Build is green

Patch application report for D7973 (id=28726)

Rebasing onto 72b6022e09...

Current branch diff-target is up to date.
Changes applied before test
commit efa8ab0f9117a86923d67119d2b1928a977e1e53
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Wed Jun 8 14:47:52 2022 +0200

    browse: Add typing to view function signatures
    
    Because parameters of view functions for browse web application were
    not typed, mypy was not processing the body of those functions and
    thus typing errors could be missed.
    
    So add typing to these function signatures and fix new mypy errors now
    new code is processed.

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

vlorentz added inline comments.
assets/src/bundles/origin/visits-reporting.js
116 ↗(On Diff #28726)

this does not seem to do the same than the old code at all. What kind of value does v.date have?

swh/web/browse/snapshot_context.py
584

why can't it be None?

1018

why can't it be None?

swh/web/browse/views/origin.py
267

[ah, I guess that answers my question above]

why do you change the format? I think it's better to keep a unix timestamp (which is a well-understood and fixed format) than rely on details of the implementation of isoformat()

swh/web/browse/snapshot_context.py
584

I should throw an exception here if the release is not found like it is done some lines above.

1018

I do not recall why i added that assert but it is not needed, will remove.

swh/web/browse/views/origin.py
267

mypy yelled at me because a string is expected here and not a float.

But I should have stringify it and convert it back to float javascript side, will update.

swh/web/browse/views/origin.py
267

or declare visit: Dict[str, Any], if the only issue is that mypy assumes it has to be Dict[str, str]

swh/web/browse/views/origin.py
267

I had to use cast but better avoiding pointeless conversions indeed.

This revision is now accepted and ready to land.Jun 9 2022, 4:06 PM

Build has FAILED

Patch application report for D7973 (id=28735)

Rebasing onto 72b6022e09...

Current branch diff-target is up to date.
Changes applied before test
commit b54d930bed54cb0a87078952cfbb3bb621290eb3
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Wed Jun 8 14:47:52 2022 +0200

    browse: Add typing to view function signatures
    
    Because parameters of view functions for browse web application were
    not typed, mypy was not processing the body of those functions and
    thus typing errors could be missed.
    
    So add typing to these function signatures and fix new mypy errors now
    new code is processed.

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

This revision was landed with ongoing or failed builds.Jun 9 2022, 4:38 PM
This revision was automatically updated to reflect the committed changes.