Paths

Table of Contentst

Differential D3041

browse/snapshot_context: Refactor, add type annotations and tests
ClosedPublic
Actions

Authored by anlambert on Apr 21 2020, 4:35 PM.

Details

Reviewers

ardumont

Group Reviewers

Reviewers

Maniphest Tasks

T2360: Reorganize and refactor snapshot browsing related code

Commits

rDWAPPS40ac1374bc4a: browse/snapshot_context: Refactor, add type annotations and tests

Summary

On the road to T2342, second part of T2360.

Slightly refactor and improve the swh.web.browse.snapshot_context module:

merge _process_snapshot_request function into get_snapshot_context one
rename some dict keys and add snapshot / origin related type annotations
cleanup and simplify get_snapshot_context implementation
update some docstrings
add new exhaustive tests for get_snapshot_context function

That diff is quite large as I performed some keys renaming in dictionaries spanning
numerous files. I have also added mising tests.

Diff Detail

Repository

rDWAPPS Web applications

Branch

snapshot-context-refactor

Lint

No Linters Available

Unit

No Unit Test Coverage

Build Status

Buildable 11956
Build 18125: Phabricator diff pipeline on jenkins	Jenkins console · Jenkins
Build 18124: arc lint + arc unit

Event Timeline

anlambert created this revision.Apr 21 2020, 4:35 PM

Herald added a reviewer: Reviewers. · View Herald TranscriptApr 21 2020, 4:35 PM

anlambert edited the summary of this revision. (Show Details)Apr 21 2020, 4:36 PM

anlambert edited the summary of this revision. (Show Details)

Build is green

Patch application report for D3041 (id=10807)

Rebasing onto 88847f09e8...

Current branch diff-target is up to date.

Changes applied before test

commit a71f64e0d19fd6fbe1c0a8e6105c9fc25eefae02
Author: Antoine Lambert <antoine.lambert@inria.fr>
Date:   Wed Apr 15 13:47:21 2020 +0200

    browse/snapshot_context: Refactor, add type annotations and tests
    
    Slightly refactor and improve the swh.web.browse.snapshot_context module:
    
      - merge _process_snapshot_request function into get_snapshot_context one
    
      - rename some dict keys and add snapshot / origin related type annotations
    
      - cleanup and simplify get_snapshot_context implementation
    
      - update some docstrings
    
      - add new exhaustive tests for get_snapshot_context function
    
    Closes T2360

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

Harbormaster completed remote builds in B11956: Diff 10807.Apr 21 2020, 4:44 PM

ardumont added a subscriber: ardumont.Apr 21 2020, 5:07 PM

ardumont added inline comments.

swh/web/browse/snapshot_context.py
644	It's retrieving the GET query parameters, right? (i forgot ;)

I think it's fine :D

It's "slightly" big ;)

This revision is now accepted and ready to land.Apr 21 2020, 5:15 PM

In D3041#73696, @ardumont wrote:

I think it's fine :D

It's "slightly" big ;)

Yes sorry, I started playing with mypy and had to modify quite a lot of files ...
This is mostly code readability improvements, some slight reorganization and code duplication removal.

I could have split it in multiple parts (notably the origin visit stuffs) but snapshots, origins and visits are so tightly linked
so I processed all of them regarding type annotations in one batch. Plus I would like to advance on T2342 as it is quite urgent for Roberto.

swh/web/browse/snapshot_context.py
644	Yes it is ;-)

anlambert mentioned this in T2353: Improve snapshot browsing when error occurs.Apr 21 2020, 5:43 PM

Update: Do not filtered out branches with a non resolvable revision (restore previous behavior)

Build is green

Patch application report for D3041 (id=10808)

Rebasing onto 88847f09e8...

Current branch diff-target is up to date.

Changes applied before test

commit 40ac1374bc4aed7b3d3c7f48f2f8dc1170446f28
Author: Antoine Lambert <antoine.lambert@inria.fr>
Date:   Wed Apr 15 13:47:21 2020 +0200

    browse/snapshot_context: Refactor, add type annotations and tests
    
    Slightly refactor and improve the swh.web.browse.snapshot_context module:
    
      - merge _process_snapshot_request function into get_snapshot_context one
    
      - rename some dict keys and add snapshot / origin related type annotations
    
      - cleanup and simplify get_snapshot_context implementation
    
      - update some docstrings
    
      - add new exhaustive tests for get_snapshot_context function
    
    Closes T2360

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

Harbormaster completed remote builds in B11958: Diff 10808.Apr 21 2020, 6:20 PM

Closed by commit rDWAPPS40ac1374bc4a: browse/snapshot_context: Refactor, add type annotations and tests (authored by anlambert). · Explain WhyApr 21 2020, 6:58 PM

This revision was automatically updated to reflect the committed changes.

anlambert added a commit: rDWAPPS40ac1374bc4a: browse/snapshot_context: Refactor, add type annotations and tests.