Paths

Table of Contentst

Differential D7973

browse: Add typing to view function signatures
ClosedPublic
Actions

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

Details

Reviewers

vlorentz

Group Reviewers

Reviewers

Commits

rDWAPPSb54d930bed54: browse: Add typing to view function signatures

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

anlambert created this revision.Jun 8 2022, 3:55 PM

Herald added a reviewer: Reviewers. · View Herald TranscriptJun 8 2022, 3:55 PM

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!

Add more browse typing

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.

Harbormaster completed remote builds in B29807: Diff 28726.Jun 8 2022, 5:44 PM

anlambert requested review of this revision.Jun 8 2022, 5:44 PM

anlambert added a child revision: D7974: common/utils: Ensure str type for query parameters in reverse function.Jun 9 2022, 11:35 AM

vlorentz added a subscriber: vlorentz.Jun 9 2022, 3:17 PM

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()`

anlambert added inline comments.Jun 9 2022, 3:36 PM

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.

vlorentz added inline comments.Jun 9 2022, 3:44 PM

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]`

anlambert added inline comments.Jun 9 2022, 4:02 PM

swh/web/browse/views/origin.py
267	I had to use `cast` but better avoiding pointeless conversions indeed.

Address @vlorentz comments

vlorentz accepted this revision.Jun 9 2022, 4:06 PM

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

Harbormaster failed remote builds in B29816: Diff 28735!Jun 9 2022, 4:26 PM

This revision was landed with ongoing or failed builds.Jun 9 2022, 4:38 PM

Closed by commit rDWAPPSb54d930bed54: browse: Add typing to view function signatures (authored by anlambert). · Explain Why

This revision was automatically updated to reflect the committed changes.

anlambert added a commit: rDWAPPSb54d930bed54: browse: Add typing to view function signatures.