Page MenuHomeSoftware Heritage

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

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

Details

Summary

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

Depends on D4959

Diff Detail

Event Timeline

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.

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

Thanks.

Adapt according to review

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.

swh/lister/gitlab/lister.py
113

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

168

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

swh/lister/gitlab/lister.py
168

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

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)

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

swh/lister/gitlab/tests/test_lister.py
225

should not end with / now ;-)

This revision is now accepted and ready to land.Jan 27 2021, 7:10 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

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 ;)

Adapt according to suggestions

  • make it parametric on both case
  • fix test

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

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

Build has FAILED

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

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.