Page MenuHomeSoftware Heritage
Differential D4957

gitlab: Deal with missing or trailing / in url input
ClosedPublic
Actions

Authored by ardumont on Jan 27 2021, 6:02 PM.

Details

Reviewers
anlambert
Group Reviewers
Reviewers
Commits
rDLS72be074a795e: gitlab: Deal with missing or trailing / in url input
Summary

so sysadm are allowed to miss a / in the url ¯\_(ツ)_/¯

Depends on D4959

Diff Detail

Repository
rDLS Listers
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

ardumont created this revision.Jan 27 2021, 6:02 PM
Herald added a reviewer: Reviewers. · View Herald TranscriptJan 27 2021, 6:02 PM
swh-public-ci added a comment.Jan 27 2021, 6:05 PM

Build is green

Patch application report for D4957 (id=17681)

Rebasing onto cbd2cce339...

Current branch diff-target is up to date.
Changes applied before test
commit 9fac58d897c008d10002a7b520367f47dabecb0f
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Wed Jan 27 18:02:08 2021 +0100

    gitlab: Allow sysadm to forget the trailing /

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

Harbormaster completed remote builds in B18822: Diff 17681.Jan 27 2021, 6:05 PM
ardumont requested review of this revision.Jan 27 2021, 6:05 PM
anlambert requested changes to this revision.Jan 27 2021, 6:11 PM
anlambert added a subscriber: anlambert.

This would be cleaner to process the url in the lister constructor instead (same as in debian one).

And it misses a test ;-)

This revision now requires changes to proceed.Jan 27 2021, 6:11 PM
ardumont updated this revision to Diff 17682.Jan 27 2021, 6:38 PM

Thanks.

Adapt according to review

swh-public-ci added a comment.Jan 27 2021, 6:41 PM

Build is green

Patch application report for D4957 (id=17682)

Rebasing onto cbd2cce339...

Current branch diff-target is up to date.
Changes applied before test
commit 2065fb167d07b072c87d739c23f9f2c645d97fd0
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Wed Jan 27 18:02:08 2021 +0100

    gitlab: Allow sysadm to forget the trailing /

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

Harbormaster completed remote builds in B18823: Diff 17682.Jan 27 2021, 6:41 PM
anlambert added inline comments.Jan 27 2021, 6:42 PM
swh/lister/gitlab/lister.py
109–112

for your case, you could use url=url.rstrip("/") and avoid doing that in the page_url method

167

would be more readable without the rstrip here, see above comment

ardumont added inline comments.Jan 27 2021, 7:02 PM
swh/lister/gitlab/lister.py
167

i forget to drop it ¯\_(ツ)_/¯

ardumont updated this revision to Diff 17684.Jan 27 2021, 7:05 PM

Thanks again!

Harbormaster completed remote builds in B18825: Diff 17684.Jan 27 2021, 7:05 PM
ardumont updated this revision to Diff 17685.Jan 27 2021, 7:06 PM

Use the right repository...

(I need to stop working... and making a mess ;)

ardumont retitled this revision from gitlab: Allow sysadm to forget the trailing / to gitlab: Deal with missing or trailing / in url input.Jan 27 2021, 7:07 PM
ardumont edited the summary of this revision. (Show Details)
swh-public-ci added a comment.Jan 27 2021, 7:09 PM

Build has FAILED

Patch application report for D4957 (id=17685)

Rebasing onto cbd2cce339...

Current branch diff-target is up to date.
Changes applied before test
commit 260c825953f6b5fe483d1cd408f86effe55ab71d
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Wed Jan 27 18:02:08 2021 +0100

    gitlab: Deal with missing or trailing / in url input

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

Harbormaster failed remote builds in B18826: Diff 17685!Jan 27 2021, 7:10 PM
anlambert added inline comments.Jan 27 2021, 7:10 PM
swh/lister/gitlab/tests/test_lister.py
225

should not end with / now ;-)

anlambert accepted this revision.Jan 27 2021, 7:10 PM
This revision is now accepted and ready to land.Jan 27 2021, 7:10 PM
anlambert added inline comments.Jan 27 2021, 7:13 PM
swh/lister/gitlab/tests/test_lister.py
225

You should test URLs with and without trailing slash to ensure both can be passed as lister parameter

ardumont added inline comments.Jan 28 2021, 9:12 AM
swh/lister/gitlab/tests/test_lister.py
225

lol, yeah, thanks

now that i have slept a bit, i shall be able to stop doing back and forth updates on this diff (hopefully ;)

ardumont updated this revision to Diff 17689.EditedJan 28 2021, 9:31 AM

Adapt according to suggestions

  • make it parametric on both case
  • fix test
ardumont marked 4 inline comments as done.Jan 28 2021, 9:32 AM
swh-public-ci added a comment.Jan 28 2021, 9:33 AM

Build has FAILED

Patch application report for D4957 (id=17689)

Rebasing onto 17b0e7af26...

Current branch diff-target is up to date.
Changes applied before test
commit 8aafe41b9f6c9e284e2046f42c285ec4bd27aed4
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Wed Jan 27 18:02:08 2021 +0100

    gitlab: Deal with missing or trailing / in url input

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

Harbormaster failed remote builds in B18830: Diff 17689!Jan 28 2021, 9:33 AM
swh-public-ci added a comment.Jan 28 2021, 9:47 AM

Build has FAILED

Patch application report for D4957 (id=17689)

Rebasing onto 17b0e7af26...

Current branch diff-target is up to date.
Changes applied before test
commit 8aafe41b9f6c9e284e2046f42c285ec4bd27aed4
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Wed Jan 27 18:02:08 2021 +0100

    gitlab: Deal with missing or trailing / in url input

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

ardumont added a comment.Jan 28 2021, 10:39 AM

Build has FAILED

unrelated to this diff, a new amqp release 5.0.4 broke the build.

ardumont edited the summary of this revision. (Show Details)Jan 28 2021, 10:50 AM
ardumont added a parent revision: D4959: tox.ini: Work around build failure due to upstream release.
ardumont updated this revision to Diff 17692.Jan 28 2021, 11:12 AM

Rebase

swh-public-ci added a comment.Jan 28 2021, 11:16 AM

Build is green

Patch application report for D4957 (id=17692)

Rebasing onto 3bede83d0f...

Current branch diff-target is up to date.
Changes applied before test
commit 72be074a795e4ab57f9aaa1c75daf7b938c2f16d
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Wed Jan 27 18:02:08 2021 +0100

    gitlab: Deal with missing or trailing / in url input

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

Harbormaster completed remote builds in B18833: Diff 17692.Jan 28 2021, 11:16 AM
Closed by commit rDLS72be074a795e: gitlab: Deal with missing or trailing / in url input (authored by ardumont). · Explain WhyJan 28 2021, 11:21 AM
This revision was automatically updated to reflect the committed changes.
ardumont added a commit: rDLS72be074a795e: gitlab: Deal with missing or trailing / in url input.

Revision Contents
Changeset List

PathSize
swh/
lister/
gitlab/
7 lines
tests/
10 lines

Diff 17693

View Options

swh/lister/gitlab/lister.py

Loading...
View Options

swh/lister/gitlab/tests/test_lister.py

Loading...
About · Developer information · Legal