Skip to content

perf(gitlab): avoid redundant project fetches for file reads - #3247

Merged
IsmaelMartinez merged 1 commit into
The-PR-Agent:mainfrom
PeterDaveHello:perf/gitlab-lazy-file-project
Sep 10, 2026
Merged

IsmaelMartinez merged 1 commit into
The-PR-Agent:mainfrom
PeterDaveHello:perf/gitlab-lazy-file-project

Conversation

@PeterDaveHello

Copy link
Copy Markdown
Contributor

Summary

Use a lazy python-gitlab project handle in GitLabProvider.get_pr_file_content() so each file-content lookup no longer fetches project metadata first.

projects.get(id) performs a project API request before files.get(...); this path only needs the project identifier to address the repository file endpoint. projects.get(id, lazy=True) preserves the same file read while avoiding that redundant round trip.

Scope

  • GitLab provider only.
  • No cache or lifecycle changes.
  • Missing-file and decoding behavior stay unchanged.
  • Adds a focused regression test that asserts the lazy project lookup and file request arguments.

Expected impact

For every fully loaded GitLab file revision, this removes one project-metadata request. A normal full diff loads both base and head content for each eligible file, so the file-content phase drops from two HTTP requests per revision (project metadata + file) to one (file only).

Validation

CI/unit tests will validate the focused regression and full suite on this branch.

@qodo-code-review

qodo-code-review Bot commented Sep 9, 2026 •

Copy link
Copy Markdown
Contributor

Code Review by Qodo

Grey Divider

Sorry, something went wrong

We weren't able to complete the code review on our side. Please try again manually by commenting /agentic_review on this PR.

Grey Divider

Qodo Logo

@IsmaelMartinez IsmaelMartinez left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for this, and for a regression test that fails without the change. Approving.

Verified. A file read drops from two HTTP requests to one, and the saving scales: get_pr_file_content runs twice per file for the first fifty files of a diff. A 404, 403 or 401 on the file request returns the same empty string as main, and there is no double-encoding for namespace paths, subgroups or numeric ids. It now addresses the project by its encoded path rather than the numeric id it resolved first, the same form main already used for that lookup.

Most of the other projects.get calls here take the same one-token change: any site that only reaches .files, .mergerequests, .languages or .labels takes lazy=True unchanged. The five reading default_branch or path_with_namespace need a hoist instead, and the three in _resolve_project_path use the eager fetch as an existence probe. Worth a follow-up if you fancy it.

@IsmaelMartinez
IsmaelMartinez merged commit 1f1abd1 into The-PR-Agent:main Sep 10, 2026
5 checks passed
@PeterDaveHello
PeterDaveHello deleted the perf/gitlab-lazy-file-project branch September 18, 2026 16:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants