Skip to content

GitHubAPIClient: scope GET requests to the chosen repo#608

Merged
shtrom merged 7 commits into
zeid/bug-1989635-github-pr-pilotfrom
no-bug/scope-get-to-repo
Oct 29, 2025
Merged

GitHubAPIClient: scope GET requests to the chosen repo#608
shtrom merged 7 commits into
zeid/bug-1989635-github-pr-pilotfrom
no-bug/scope-get-to-repo

Conversation

@shtrom
Copy link
Copy Markdown
Member

@shtrom shtrom commented Oct 15, 2025

No description provided.

@shtrom shtrom requested a review from zzzeid October 15, 2025 07:20
@shtrom shtrom marked this pull request as ready for review October 15, 2025 07:20
@zzzeid zzzeid force-pushed the zeid/bug-1989963-pr-view branch 4 times, most recently from 60d0b63 to 856dffe Compare October 21, 2025 17:27
- add GitHubAPI class
- add GitHubAPIClient class
- move "get token" functionality to GitHubAPI class
- fix failing test_GitSCM_push_get_github_token
@zzzeid zzzeid force-pushed the zeid/bug-1989963-pr-view branch from 856dffe to 456ba02 Compare October 21, 2025 19:54
@shtrom shtrom force-pushed the no-bug/scope-get-to-repo branch from c232a88 to 81d2887 Compare October 22, 2025 04:42
@shtrom shtrom changed the base branch from zeid/bug-1989963-pr-view to no-bug/github-api-url-parsing October 22, 2025 04:45
@shtrom shtrom changed the base branch from no-bug/github-api-url-parsing to zeid/bug-1989963-pr-view October 22, 2025 04:47
@shtrom
Copy link
Copy Markdown
Member Author

shtrom commented Oct 22, 2025

This will need some conflict resolution after #604 is merged.

@zzzeid zzzeid force-pushed the zeid/bug-1989963-pr-view branch from 456ba02 to 69f1574 Compare October 22, 2025 13:17
Base automatically changed from zeid/bug-1989963-pr-view to zeid/bug-1989635-github-pr-pilot October 22, 2025 13:29
zzzeid
zzzeid previously requested changes Oct 24, 2025
Comment thread src/lando/utils/github.py
Comment thread src/lando/utils/github.py Outdated
Return:
dist | list: decoded JSON from the response
"""
result = self.client.get(f"{self.repo_base_url}/{path}", *args, **kwargs)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was left out of this method in case there is a non-repo scoped URL that the client needs to access (e.g., repos, users, etc...)

I think there may be a different way to do this that doesn't preclude this ability in the future (e.g., adding self._get_repo method).

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, _get_repo could work. I mainly want to avoid the duplication of the repo_base_url parameter everywhere if we can avoid it, as the Session already has this scope.

@zzzeid zzzeid force-pushed the zeid/bug-1989635-github-pr-pilot branch 4 times, most recently from 1ede676 to 7c1d6a8 Compare October 28, 2025 20:02
@shtrom shtrom force-pushed the no-bug/scope-get-to-repo branch from 14cb5c3 to 5f83909 Compare October 29, 2025 00:17
@shtrom shtrom force-pushed the no-bug/scope-get-to-repo branch from 5f83909 to 56a2af8 Compare October 29, 2025 00:21
@shtrom shtrom requested a review from zzzeid October 29, 2025 00:21
@shtrom shtrom dismissed zzzeid’s stale review October 29, 2025 00:21

merged pr-pilot pr, reverted _get changes, added _repo_get instead

Comment thread src/lando/utils/github.py
Copy link
Copy Markdown
Contributor

@zzzeid zzzeid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving this with the caveat about the one comment (re: _post method) which should be addressed before this is merged.

Comment thread src/lando/utils/github.py Outdated
Co-authored-by: Zeid <2043828+zzzeid@users.noreply.github.com>
@shtrom shtrom merged commit 3d9712a into zeid/bug-1989635-github-pr-pilot Oct 29, 2025
1 check passed
@shtrom shtrom deleted the no-bug/scope-get-to-repo branch October 29, 2025 22:41
shtrom added a commit that referenced this pull request Oct 31, 2025
zzzeid added a commit that referenced this pull request Nov 6, 2025
zzzeid added a commit that referenced this pull request Nov 6, 2025
zzzeid added a commit that referenced this pull request Nov 20, 2025
zzzeid added a commit that referenced this pull request Nov 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants