Page MenuHomeSoftware Heritage
Differential D7836

Add utility to sanitize and retrieve canonical github urls
ClosedPublic
Actions

Authored by ardumont on May 16 2022, 3:03 PM.

Details

Reviewers
anlambert
Group Reviewers
Reviewers
Maniphest Tasks
T4232: Listers: Canonicalize listed github origins
Commits
rDCORE60f384d35ad8: Add utility function to retrieve canonical github urls
Summary

Based on the work in webapp [1]

This new code is within a new arborescence as some more code will get moved alongside in
a new commit (the new session github currently in swh.lister module).

Related to T4232

[1] https://forge.softwareheritage.org/source/swh-web/browse/master/assets/src/utils/functions.js$118-160

Diff Detail

Event Timeline

ardumont created this revision.May 16 2022, 3:03 PM
Herald added a reviewer: Reviewers. · View Herald TranscriptMay 16 2022, 3:03 PM
swh-public-ci added a comment.May 16 2022, 3:05 PM

Build is green

Patch application report for D7836 (id=28302)

Rebasing onto dae963440b...

Current branch diff-target is up to date.
Changes applied before test
commit dd1158bb1f308da55e8dd852a1916d60867e839b
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Mon May 16 15:01:25 2022 +0200

    Add utility to sanitize and retrieve canonical github urls
    
    Related to T4232

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

Harbormaster completed remote builds in B29404: Diff 28302.May 16 2022, 3:05 PM
ardumont requested review of this revision.May 16 2022, 3:05 PM
anlambert added a subscriber: anlambert.May 16 2022, 3:11 PM
anlambert added inline comments.
swh/core/utils.py
219–233

I think you should add support to pass GitHub API token in Authorization HTTP header, there is good chances we hit the rate limit without it.

ardumont added inline comments.May 16 2022, 3:22 PM
swh/core/utils.py
219–233

Right, i had it in mind when i started and then I forgot it...

ardumont updated this revision to Diff 28307.May 16 2022, 3:51 PM

Move code in a dedicated module (explanation will be added in the diff description)

ardumont edited the summary of this revision. (Show Details)May 16 2022, 3:51 PM
swh-public-ci added a comment.May 16 2022, 3:53 PM

Build is green

Patch application report for D7836 (id=28307)

Rebasing onto dae963440b...

Current branch diff-target is up to date.
Changes applied before test
commit b9252925472099fe28714da568fe462a46ba70b7
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Mon May 16 15:01:25 2022 +0200

    Add utility to sanitize and retrieve canonical github urls
    
    This new code is within a new arborescence as some more code will get moved alongside in
    a new commit (the new session github currently in swh.lister module).
    
    Related to T4232

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

Harbormaster completed remote builds in B29409: Diff 28307.May 16 2022, 3:53 PM
ardumont added inline comments.May 16 2022, 3:54 PM
swh/core/utils.py
219–233

I'll plug this in another diff.
I need to refactor [1] out of swh.lister here ^.

[1] https://forge.softwareheritage.org/source/swh-lister/browse/master/swh/lister/github/utils.py

vlorentz added a subscriber: vlorentz.May 16 2022, 3:54 PM
vlorentz added inline comments.
swh/core/github/utils.py
22–26 ↗(On Diff #28307)

simpler, and also catches the "https://github.com/Foo/Bar/.git" case

ardumont added inline comments.May 16 2022, 4:04 PM
swh/core/github/utils.py
22–26 ↗(On Diff #28307)

sneaky!

ardumont added inline comments.May 16 2022, 4:12 PM
swh/core/github/utils.py
22–26 ↗(On Diff #28307)

ymmv, i don't find it easier in the end and, currently:

return re.match(r"(.*?)(.git)?/?", url.lower())[1]

fails on the first test case though:

E       AssertionError: assert '' == 'https://github.com/user/repo'
E         - https://github.com/user/repo
ardumont updated this revision to Diff 28308.May 16 2022, 4:18 PM
ardumont edited the summary of this revision. (Show Details)

Take care of the extra case not dealt with yet

I could not make the suggested improvment work though ¯\_(ツ)_/¯

anlambert accepted this revision.May 16 2022, 4:19 PM

Looks good to me.

This revision is now accepted and ready to land.May 16 2022, 4:19 PM
swh-public-ci added a comment.May 16 2022, 4:20 PM

Build is green

Patch application report for D7836 (id=28308)

Rebasing onto dae963440b...

Current branch diff-target is up to date.
Changes applied before test
commit a5ac30bc5d39e33fd5dfc4d0052d486b49d54bfb
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Mon May 16 15:01:25 2022 +0200

    Add utility to sanitize and retrieve canonical github urls
    
    This new code is within a new arborescence as some more code will get moved alongside in
    a new commit (the new session github currently in swh.lister module).
    
    Related to T4232

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

Harbormaster completed remote builds in B29410: Diff 28308.May 16 2022, 4:20 PM
ardumont updated this revision to Diff 28309.May 16 2022, 4:47 PM
  • Add missing copyright headers
  • Simplify a bit more the current sanitization code (no regexp though)
  • Improve docstring (to mention the anonymous http code <- for now)
swh-public-ci added a comment.May 16 2022, 4:49 PM

Build is green

Patch application report for D7836 (id=28309)

Rebasing onto dae963440b...

Current branch diff-target is up to date.
Changes applied before test
commit 0d2b85f7f9af818c387a8b19a8ee15be01c341e2
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Mon May 16 15:01:25 2022 +0200

    Add utility to sanitize and retrieve canonical github urls
    
    This new code is within a new arborescence as some more code will get moved alongside in
    a new commit (the new session github currently in swh.lister module).
    
    Related to T4232

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

Harbormaster completed remote builds in B29411: Diff 28309.May 16 2022, 4:49 PM
ardumont updated this revision to Diff 28310.May 16 2022, 4:51 PM

Improve docstring and commit message further

swh-public-ci added a comment.May 16 2022, 4:53 PM

Build is green

Patch application report for D7836 (id=28310)

Rebasing onto dae963440b...

Current branch diff-target is up to date.
Changes applied before test
commit 60f384d35ad8d1e1fe442ea031d63262a5cafa22
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Mon May 16 15:01:25 2022 +0200

    Add utility function to retrieve canonical github urls
    
    This new code is within a new arborescence as some more code will get moved alongside in
    a new commit (the new session github currently in swh.lister module).
    
    That current code is making anonymous requests to the github api for now.
    
    Related to T4232

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

Harbormaster completed remote builds in B29412: Diff 28310.May 16 2022, 4:53 PM
Closed by commit rDCORE60f384d35ad8: Add utility function to retrieve canonical github urls (authored by ardumont). · Explain WhyMay 16 2022, 5:00 PM
This revision was automatically updated to reflect the committed changes.
ardumont added a commit: rDCORE60f384d35ad8: Add utility function to retrieve canonical github urls.
ardumont mentioned this in T4232: Listers: Canonicalize listed github origins.May 17 2022, 10:27 AM

Revision Contents
Changeset List

PathSizeCoverage (All)Coverage (Touched)
swh/
core/
tests/
37 lines100%
Loading...
46 lines95%
Loading...

Diff 28302

View Options

swh/core/tests/test_utils.py

Loading...
View Options

swh/core/utils.py

Loading...
About · Developer information · Legal