Page MenuHomeSoftware Heritage
Differential D8752

gogs/lister: Allow public gogs instance listing
ClosedPublic
Actions

Authored by ardumont on Oct 21 2022, 6:03 PM.

Details

Reviewers
anlambert
Group Reviewers
Reviewers
Commits
rDLS8a82bbf95fd2: gogs/lister: Allow public gogs instance listing
Summary

Prior to this commit, the lister assumed authentication was required while it exists
public gogs instances which do not require it.

This also updates documentation to mention the usual api location to check when actually
triggering a listing.

Co-authored with Antoine Lambert (@anlambert) <anlambert@softwareheritage.org>

Related to infra/sysadm-environment#4644
Related to https://gitlab.softwareheritage.org/infra/sysadm-environment/-/issues/4644#note_12601

Diff Detail

Repository
rDLS Listers
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 32503
Build 50901: Phabricator diff pipeline on jenkinsJenkins console · Jenkins
Build 50900: arc lint + arc unit

Event Timeline

ardumont created this revision.Oct 21 2022, 6:03 PM
Herald added a reviewer: Reviewers. · View Herald TranscriptOct 21 2022, 6:03 PM
anlambert added inline comments.Oct 21 2022, 6:06 PM
swh/lister/gogs/lister.py
107–110

We should remove that method in that lister but also in the gitea one (which simply inherits from the gogs lister) and move the warning in the __init__ method instead.

swh-public-ci added a comment.Oct 21 2022, 6:08 PM

Build is green

Patch application report for D8752 (id=31549)

Rebasing onto 0baaf68cff...

Current branch diff-target is up to date.
Changes applied before test
commit c99ba8d5150aa09216f5a71e3d1e044930537a10
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Fri Oct 21 17:50:49 2022 +0200

    gogs/lister: Allow listing on public gogs instance
    
    Prior to this commit, the lister assumed authentication was required while it exists
    public gogs instances which do not require it.
    
    This also updates documentation to mention the usual api location to check when actually
    triggering a listing.
    
    Co-authored with Antoine Lambert (@anlambert) <anlambert@softwareheritage.org>
    
    Related to infra/sysadm-environment#4644

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

Harbormaster completed remote builds in B32503: Diff 31549.Oct 21 2022, 6:08 PM
ardumont requested review of this revision.Oct 21 2022, 6:08 PM
ardumont updated this revision to Diff 31550.Oct 21 2022, 6:10 PM
ardumont marked an inline comment as done.

Adapt according to suggestion

swh/lister/gogs/lister.py
107–110

right, done!

ardumont retitled this revision from gogs/lister: Allow listing on public gogs instance to gogs/lister: Allow public gogs instance listing.Oct 21 2022, 6:11 PM
ardumont updated this revision to Diff 31551.Oct 21 2022, 6:12 PM

Amend commit message

swh-public-ci added a comment.Oct 21 2022, 6:15 PM

Build is green

Patch application report for D8752 (id=31550)

Rebasing onto 0baaf68cff...

Current branch diff-target is up to date.
Changes applied before test
commit ac95987454d99fe42e2c3c5a3b2c30704add4dcd
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Fri Oct 21 17:50:49 2022 +0200

    gogs/lister: Allow listing on public gogs instance
    
    Prior to this commit, the lister assumed authentication was required while it exists
    public gogs instances which do not require it.
    
    This also updates documentation to mention the usual api location to check when actually
    triggering a listing.
    
    Co-authored with Antoine Lambert (@anlambert) <anlambert@softwareheritage.org>
    
    Related to infra/sysadm-environment#4644

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

Harbormaster completed remote builds in B32504: Diff 31550.Oct 21 2022, 6:15 PM
swh-public-ci added a comment.Oct 21 2022, 6:17 PM

Build is green

Patch application report for D8752 (id=31551)

Rebasing onto 0baaf68cff...

Current branch diff-target is up to date.
Changes applied before test
commit 05ea67779413c41c7ec6898ff74b568336849ee6
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Fri Oct 21 17:50:49 2022 +0200

    gogs/lister: Allow public gogs instance listing
    
    Prior to this commit, the lister assumed authentication was required. It exists public
    gogs instances which do not require it.
    
    This also updates documentation to mention the usual api location. This is useful when
    people wants to actually trigger a listing as a pre-check flight.
    
    Co-authored with Antoine Lambert (@anlambert) <anlambert@softwareheritage.org>.
    
    Related to infra/sysadm-environment#4644

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

Harbormaster completed remote builds in B32505: Diff 31551.Oct 21 2022, 6:17 PM
anlambert added inline comments.Oct 21 2022, 6:18 PM
swh/lister/gogs/lister.py
107–110

You forgot to commit the swh/lister/gitea/lister.py file I think.

ardumont updated this revision to Diff 31552.Oct 21 2022, 6:22 PM

Right you are

anlambert accepted this revision.Oct 21 2022, 6:27 PM

LGTM

This revision is now accepted and ready to land.Oct 21 2022, 6:27 PM
swh-public-ci added a comment.Oct 21 2022, 6:27 PM

Build is green

Patch application report for D8752 (id=31552)

Rebasing onto 0baaf68cff...

Current branch diff-target is up to date.
Changes applied before test
commit 8a82bbf95fd23a19022541928f54057ed605ed1e
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Fri Oct 21 17:50:49 2022 +0200

    gogs/lister: Allow public gogs instance listing
    
    Prior to this commit, the lister assumed authentication was required. It exists public
    gogs instances which do not require it.
    
    This also updates documentation to mention the usual api location. This is useful when
    people wants to actually trigger a listing as a pre-check flight.
    
    This drops repetitive instruction in the gitea lister as well.
    
    Co-authored with Antoine Lambert (@anlambert) <anlambert@softwareheritage.org>.
    
    Related to infra/sysadm-environment#4644

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

Harbormaster completed remote builds in B32506: Diff 31552.Oct 21 2022, 6:27 PM
Closed by commit rDLS8a82bbf95fd2: gogs/lister: Allow public gogs instance listing (authored by ardumont). · Explain WhyOct 24 2022, 11:41 AM
This revision was automatically updated to reflect the committed changes.
ardumont added a commit: rDLS8a82bbf95fd2: gogs/lister: Allow public gogs instance listing.

Revision Contents
Changeset List

PathSizeCoverage (All)Coverage (Touched)
swh/
lister/
gogs/
18 lines93%100%
tests/
8 lines100%100%

Diff 31549

View Options

swh/lister/gogs/lister.py

Show First 20 Lines • Show All 44 Lines • ▼ Show 20 Lines
class GogsLister(Lister[GogsListerState, GogsListerPage]): class GogsLister(Lister[GogsListerState, GogsListerPage]):
"""List origins from the Gogs """List origins from the Gogs
Gogs API documentation: https://github.com/gogs/docs-api Gogs API documentation: https://github.com/gogs/docs-api
The API is protected behind authentication so credentials/API tokens The API may be protected behind authentication so credentials/API tokens can be
are mandatory. It supports pagination and provides next page URL provided.
through the 'next' value of the 'Link' header. The default value for
page size ('limit') is 10 but the maximum allowed value is 50. The lister supports pagination and provides next page URL through the 'next' value
of the 'Link' header. The default value for page size ('limit') is 10 but the
maximum allowed value is 50.
Api can usually be found at the location: https://<host>/api/v1/repos/search
""" """
LISTER_NAME = "gogs" LISTER_NAME = "gogs"
VISIT_TYPE = "git" VISIT_TYPE = "git"
REPO_LIST_PATH = "repos/search" REPO_LIST_PATH = "repos/search"
Show All 29 Lines ):
# Raises an error on Gogs, or a warning on Gitea # Raises an error on Gogs, or a warning on Gitea
self.on_anonymous_mode() self.on_anonymous_mode()
self.session.headers.update({"Accept": "application/json"}) self.session.headers.update({"Accept": "application/json"})
if self.api_token: if self.api_token:
self.session.headers["Authorization"] = f"token {self.api_token}" self.session.headers["Authorization"] = f"token {self.api_token}"
def on_anonymous_mode(self): def on_anonymous_mode(self):
raise ValueError("No credentials or API token provided") logger.warning(
"No authentication token set in configuration, using anonymous mode"
)
anlambertUnsubmitted
Done
Inline Actions

We should remove that method in that lister but also in the gitea one (which simply inherits from the gogs lister) and move the warning in the __init__ method instead.

anlambert: We should remove that method in that lister but also in the gitea one (which simply inherits…
ardumontAuthorUnsubmitted
Done
Inline Actions

right, done!

ardumont: right, done!
anlambertUnsubmitted
Not Done
Inline Actions

You forgot to commit the swh/lister/gitea/lister.py file I think.

anlambert: You forgot to commit the `swh/lister/gitea/lister.py` file I think.
def state_from_dict(self, d: Dict[str, Any]) -> GogsListerState: def state_from_dict(self, d: Dict[str, Any]) -> GogsListerState:
return GogsListerState(**d) return GogsListerState(**d)
def state_to_dict(self, state: GogsListerState) -> Dict[str, Any]: def state_to_dict(self, state: GogsListerState) -> Dict[str, Any]:
return asdict(state) return asdict(state)
def page_request( def page_request(
Show All 36 Lines def get_pages(self) -> Iterator[GogsListerPage]:
body, links = self.page_request( body, links = self.page_request(
next_link, {**self.query_params, "page": page_id} next_link, {**self.query_params, "page": page_id}
) )
while next_link is not None: while next_link is not None:
repos = self.extract_repos(body) repos = self.extract_repos(body)
assert len(links) > 0, "API changed: no Link header found"
if "next" in links: if "next" in links:
next_link = links["next"]["url"] next_link = links["next"]["url"]
else: else:
next_link = None # Happens for the last page next_link = None # Happens for the last page
yield GogsListerPage(repos=repos, next_link=next_link) yield GogsListerPage(repos=repos, next_link=next_link)
if next_link is not None: if next_link is not None:
▲ Show 20 Lines • Show All 42 Lines • Show Last 20 Lines
View Options

swh/lister/gogs/tests/test_lister.py

Show First 20 Lines • Show All 133 Lines • ▼ Show 20 Lines):
assert ( assert (
lister.get_state_from_scheduler().last_seen_next_link == P3 lister.get_state_from_scheduler().last_seen_next_link == P3
) # P3 didn't provide any next link so it remains the last_seen_next_link ) # P3 didn't provide any next link so it remains the last_seen_next_link
def test_gogs_auth_instance( def test_gogs_auth_instance(
swh_scheduler, requests_mock, trygogs_p1, trygogs_p2, trygogs_p3_empty swh_scheduler, requests_mock, trygogs_p1, trygogs_p2, trygogs_p3_empty
): ):
"""Covers token authentication, token from credentials, """Covers without authentication, token authentication, token from credentials,
instance inference from URL.""" instance inference from URL."""
api_token = "secret" api_token = "secret"
instance = "try_gogs" instance = "try_gogs"
# Test lister initialization without api_token or credentials: # Test lister initialization without api_token or credentials:
with pytest.raises(ValueError, match="No credentials or API token provided"):
kwargs1 = dict(url=TRY_GOGS_URL, instance=instance) kwargs1 = dict(url=TRY_GOGS_URL, instance=instance)
GogsLister(scheduler=swh_scheduler, **kwargs1) lister = GogsLister(scheduler=swh_scheduler, **kwargs1)
assert "Authorization" not in lister.session.headers
# Test lister initialization using api_token: # Test lister initialization using api_token:
kwargs2 = dict(url=TRY_GOGS_URL, api_token=api_token, instance=instance) kwargs2 = dict(url=TRY_GOGS_URL, api_token=api_token, instance=instance)
lister = GogsLister(scheduler=swh_scheduler, **kwargs2) lister = GogsLister(scheduler=swh_scheduler, **kwargs2)
assert lister.session.headers["Authorization"].lower() == "token %s" % api_token assert lister.session.headers["Authorization"].lower() == "token %s" % api_token
# Test lister initialization with credentials and run it: # Test lister initialization with credentials and run it:
creds = {"gogs": {instance: [{"username": "u", "password": api_token}]}} creds = {"gogs": {instance: [{"username": "u", "password": api_token}]}}
▲ Show 20 Lines • Show All 171 Lines • Show Last 20 Lines
About · Developer information · Legal