Page MenuHomeSoftware Heritage
Differential D6615

Deprecate /origin/content route
ClosedPublic
Actions

Authored by jayeshv on Nov 5 2021, 1:25 PM.

Details

Reviewers
anlambert
Group Reviewers
Reviewers
Maniphest Tasks
T3608: Deprecate most of the /browse/origin/.* URLs
T3697: Deprecate origin content routes
Commits
rDWAPPSc491d4e46a3f: Deprecate /origin/content and /snapshot/content routes
Summary

This fix deprecates and redirects /origin/content?origin_url=<url> and
/origin/<url>/content to /content/<sha1_git>

  • test fixes
  • documentation changes

Related to T3608
Fixes T3697

Diff Detail

Repository
rDWAPPS Web applications
Branch
origin-content
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 25005
Build 39072: Phabricator diff pipeline on jenkinsJenkins console · Jenkins
Build 39071: arc lint + arc unit

Event Timeline

jayeshv created this revision.Nov 5 2021, 1:25 PM
Herald added a reviewer: Reviewers. · View Herald TranscriptNov 5 2021, 1:25 PM
jayeshv retitled this revision from Deprecate /origin/content route to [WIP] Deprecate /origin/content route.Nov 5 2021, 1:26 PM
jayeshv edited the summary of this revision. (Show Details)
swh-public-ci added a comment.Nov 5 2021, 1:47 PM

Build has FAILED

Patch application report for D6615 (id=24018)

Rebasing onto 1e106c798e...

Current branch diff-target is up to date.
Changes applied before test
commit 32a49a41e9fb7cb0bbbe9ec1da557121957ef5ec
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Fri Nov 5 13:21:35 2021 +0100

    Deprecate /origin/content route
    
    This fix deprecates and redirects /origin/content?origin_url=<url> and
    /origin/<url>/content to /content/<sha1_git>
    - test fixes
    - documentation changes
    
    Related to T3608
    Fixes T3697

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

Harbormaster returned this revision to the author for changes because remote builds failed.Nov 5 2021, 1:47 PM
Harbormaster failed remote builds in B24891: Diff 24018!
jayeshv updated this revision to Diff 24122.Nov 10 2021, 2:57 PM

redirecting snapshot/content route

swh-public-ci added a comment.Nov 10 2021, 3:19 PM

Build has FAILED

Patch application report for D6615 (id=24122)

Rebasing onto 1e106c798e...

Current branch diff-target is up to date.
Changes applied before test
commit d9fd08080fa26486d8ab4c13048fb056d367e65b
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Wed Nov 10 14:53:51 2021 +0100

    Deprecate /origin/content and /snapshot/content routes
    
    This fix deprecates and redirects /origin/content?origin_url=<url>,
    /origin/<url>/content, /snapshot/<id>/content and /snapshot/<id>/content/<path>
    to /content/<sha1_git>
    
    - test fixes
    - documentation changes
    
    Related to T3608
    Fixes T3697

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

Harbormaster returned this revision to the author for changes because remote builds failed.Nov 10 2021, 3:19 PM
Harbormaster failed remote builds in B25001: Diff 24122!
jayeshv updated this revision to Diff 24126.Nov 10 2021, 4:07 PM

guided tour fix

swh-public-ci added a comment.Nov 10 2021, 4:34 PM

Build was aborted

Patch application report for D6615 (id=24126)

Rebasing onto 1e106c798e...

Current branch diff-target is up to date.
Changes applied before test
commit 0164aa332d18ae8c9af23c2c94c7b3b0dfbd6355
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Wed Nov 10 16:06:11 2021 +0100

    Deprecate /origin/content and /snapshot/content routes
    
    This fix deprecates and redirects /origin/content?origin_url=<url>,
    /origin/<url>/content, /snapshot/<id>/content and /snapshot/<id>/content/<path>
    to /content/<sha1_git>
    
    - test fixes
    - documentation changes
    
    Related to T3608
    Fixes T3697

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

Harbormaster returned this revision to the author for changes because remote builds failed.Nov 10 2021, 4:34 PM
Harbormaster failed remote builds in B25005: Diff 24126!
swh-public-ci added a comment.Nov 10 2021, 4:41 PM

Build has FAILED

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

jayeshv updated this revision to Diff 24131.Nov 15 2021, 12:21 PM

unit tests

swh-public-ci added a comment.Nov 15 2021, 12:36 PM

Build has FAILED

Patch application report for D6615 (id=24131)

Rebasing onto fe8c3b38bb...

First, rewinding head to replay your work on top of it...
Applying: Deprecate /origin/content and /snapshot/content routes
Changes applied before test
commit 191063ec569d568de13f1d8619ec3ae5c850b76b
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Mon Nov 15 12:20:42 2021 +0100

    Deprecate /origin/content and /snapshot/content routes
    
    This fix deprecates and redirects /origin/content?origin_url=<url>,
    /origin/<url>/content, /snapshot/<id>/content and /snapshot/<id>/content/<path>
    to /content/<sha1_git>
    
    - test fixes
    - documentation changes
    
    Related to T3608
    Fixes T3697

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

Harbormaster returned this revision to the author for changes because remote builds failed.Nov 15 2021, 12:36 PM
Harbormaster failed remote builds in B25011: Diff 24131!
jayeshv updated this revision to Diff 24135.Nov 15 2021, 3:57 PM

rst fix

swh-public-ci added a comment.Nov 15 2021, 4:13 PM

Build is green

Patch application report for D6615 (id=24135)

Rebasing onto e2659cfed5...

Current branch diff-target is up to date.
Changes applied before test
commit cfb782b486f2df1d92f747614045ec68a6c6abdc
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Mon Nov 15 15:57:25 2021 +0100

    Deprecate /origin/content and /snapshot/content routes
    
    This fix deprecates and redirects /origin/content?origin_url=<url>,
    /origin/<url>/content, /snapshot/<id>/content and /snapshot/<id>/content/<path>
    to /content/<sha1_git>
    
    - test fixes
    - documentation changes
    
    Related to T3608
    Fixes T3697

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

Harbormaster completed remote builds in B25015: Diff 24135.Nov 15 2021, 4:13 PM
jayeshv requested review of this revision.Nov 15 2021, 4:13 PM
jayeshv updated this revision to Diff 24140.Nov 16 2021, 12:00 PM

doc updates

swh-public-ci added a comment.Nov 16 2021, 12:16 PM

Build is green

Patch application report for D6615 (id=24140)

Rebasing onto e2659cfed5...

Current branch diff-target is up to date.
Changes applied before test
commit f9579e8e57430c153bc22092db3f8ccf2630de2c
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Tue Nov 16 11:59:12 2021 +0100

    Deprecate /origin/content and /snapshot/content routes
    
    This fix deprecates and redirects /origin/content?origin_url=<url>,
    /origin/<url>/content, /snapshot/<id>/content and /snapshot/<id>/content/<path>
    to /content/<sha1_git>
    
    - test fixes
    - documentation changes
    
    Related to T3608
    Fixes T3697

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

Harbormaster completed remote builds in B25020: Diff 24140.Nov 16 2021, 12:16 PM
jayeshv updated this revision to Diff 24141.Nov 16 2021, 12:17 PM

Minor changes

jayeshv retitled this revision from [WIP] Deprecate /origin/content route to Deprecate /origin/content route.Nov 16 2021, 12:18 PM
jayeshv edited the summary of this revision. (Show Details)
swh-public-ci added a comment.Nov 16 2021, 12:34 PM

Build is green

Patch application report for D6615 (id=24141)

Rebasing onto e2659cfed5...

Current branch diff-target is up to date.
Changes applied before test
commit b317e5ab617f0ea3bb9c7895c6ec119eb490c0b9
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Tue Nov 16 12:16:14 2021 +0100

    Deprecate /origin/content and /snapshot/content routes
    
    This fix deprecates and redirects /origin/content?origin_url=<url>,
    /origin/<url>/content, /snapshot/<id>/content and /snapshot/<id>/content/<path>
    to /content/<sha1_git>
    
    - test fixes
    - documentation changes
    
    Related to T3608
    Fixes T3697

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

Harbormaster completed remote builds in B25021: Diff 24141.Nov 16 2021, 12:34 PM
jayeshv updated this revision to Diff 24156.Nov 16 2021, 4:25 PM

Missing tests

swh-public-ci added a comment.Nov 16 2021, 4:42 PM

Build is green

Patch application report for D6615 (id=24156)

Rebasing onto 189d204e9b...

First, rewinding head to replay your work on top of it...
Applying: Deprecate /origin/content and /snapshot/content routes
Changes applied before test
commit fa8be2b76b411ebe3825abeea91a2cad7f15a6ae
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Tue Nov 16 16:24:53 2021 +0100

    Deprecate /origin/content and /snapshot/content routes
    
    This fix deprecates and redirects /origin/content?origin_url=<url>,
    /origin/<url>/content, /snapshot/<id>/content and /snapshot/<id>/content/<path>
    to /content/
    
    - test fixes
    - documentation changes
    
    Related to T3608
    Fixes T3697

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

Harbormaster completed remote builds in B25036: Diff 24156.Nov 16 2021, 4:42 PM
anlambert requested changes to this revision.Nov 17 2021, 12:09 PM
anlambert added a subscriber: anlambert.

Overall looks good to me ! I added a couple of nitpick comments to handle before landing this.

assets/src/bundles/guided_tour/index.js
69

Why did you not use ${Urls.browse_content()}?origin_url=${originUrl}&path=Example/example.c here ?

docs/uri-scheme-browse-content.rst
43 ↗(On Diff #24156)

Could you add the doc (copy paste from below) for the following accepted query parameters: origin_url, snapshot, branch, release, timestamp ?

102–103 ↗(On Diff #24156)

this is optional too, HEAD branch will be used if not provided.

104–105 ↗(On Diff #24156)

also optional

swh/web/browse/views/content.py
175

The path query parameter must be provided.

179

The origin_url or snapshot query parameters must be provided.

210–211

this case happens when redirected either from /browse/origin/content or /browse/snapshot/content

swh/web/common/utils.py
421–426

Too many lines in that docstring imho, you should make it more compact as you can use up to 88 columns per line.

This revision now requires changes to proceed.Nov 17 2021, 12:09 PM
jayeshv updated this revision to Diff 24166.Nov 17 2021, 1:37 PM

Review changes

swh-public-ci added a comment.Nov 17 2021, 1:59 PM

Build was aborted

Patch application report for D6615 (id=24166)

Rebasing onto 9c874cd715...

Current branch diff-target is up to date.
Changes applied before test
commit 8bbca7dde29426d317818f0eb313edc949c898f3
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Wed Nov 17 13:33:24 2021 +0100

    Deprecate /origin/content and /snapshot/content routes
    
    This fix deprecates and redirects /origin/content?origin_url=<url>,
    /origin/<url>/content, /snapshot/<id>/content and /snapshot/<id>/content/<path>
    to /content/
    
    - test fixes
    - documentation changes
    
    Related to T3608
    Fixes T3697

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

Harbormaster failed remote builds in B25047: Diff 24166!Nov 17 2021, 1:59 PM
jayeshv updated this revision to Diff 24169.Nov 17 2021, 2:08 PM

cypress test fix

jayeshv marked 7 inline comments as done.Nov 17 2021, 2:14 PM
jayeshv added inline comments.
assets/src/bundles/guided_tour/index.js
69

This is going to an infinite redirect loop without the contentSha1. It is because the js wrapper function that controls the guided tour page is not designed to work with server redirects. It will restart the js script each time it reloads and continue the loop.
Other possible fix is to explicitly pop guided_tour param from query during redirect.

anlambert added inline comments.Nov 17 2021, 2:19 PM
assets/src/bundles/guided_tour/index.js
69

Just figured that out too, let's keep it that way then.

swh-public-ci added a comment.Nov 17 2021, 2:24 PM

Build is green

Patch application report for D6615 (id=24169)

Rebasing onto 9c874cd715...

Current branch diff-target is up to date.
Changes applied before test
commit c491d4e46a3f3c96c2e2831515d77e0ac92b90ff
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Wed Nov 17 14:07:27 2021 +0100

    Deprecate /origin/content and /snapshot/content routes
    
    This fix deprecates and redirects /origin/content?origin_url=<url>,
    /origin/<url>/content, /snapshot/<id>/content and /snapshot/<id>/content/<path>
    to /content/
    
    - test fixes
    - documentation changes
    
    Related to T3608
    Fixes T3697

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

Harbormaster completed remote builds in B25050: Diff 24169.Nov 17 2021, 2:24 PM
anlambert accepted this revision.Nov 17 2021, 2:38 PM

Bye bye duplicated code related to browsing contents, thanks a lot for that !

This revision is now accepted and ready to land.Nov 17 2021, 2:38 PM
Closed by commit rDWAPPSc491d4e46a3f: Deprecate /origin/content and /snapshot/content routes (authored by jayeshv). · Explain WhyNov 17 2021, 2:41 PM
This revision was automatically updated to reflect the committed changes.
jayeshv added a commit: rDWAPPSc491d4e46a3f: Deprecate /origin/content and /snapshot/content routes.

Revision Contents
Changeset List

PathSize
assets/
src/
bundles/
guided_tour/
4 lines
swh/
web/
browse/
403 lines
views/
45 lines
46 lines
28 lines
common/
14 lines
tests/
browse/
views/
19 lines
314 lines

Diff 24126

View Options

assets/src/bundles/guided_tour/index.js

Loading...
View Options

swh/web/browse/snapshot_context.py

Loading...
View Options

swh/web/browse/views/content.py

Loading...
View Options

swh/web/browse/views/origin.py

Loading...
View Options

swh/web/browse/views/snapshot.py

Loading...
View Options

swh/web/common/utils.py

Loading...
View Options

swh/web/tests/browse/views/test_content.py

Loading...
View Options

swh/web/tests/browse/views/test_origin.py

Loading...
About · Developer information · Legal