Issue 564: add size filtering and typed min access level#891
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds GitLab project filtering by minimum access level and by project storage size; exports an AccessLevel enum; threads minAccessLevel and statistics into project-listing options; implements getProjectSizeBytes and size-based exclusion logic; updates schemas/types; adds unit tests for size exclusion. Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 🧹 Recent nitpick comments
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
WilliamZhang9
force-pushed
the
u/williamzhang/issue_564_add_repo_size_exclude_option_for_gitlab
branch
3 times, most recently
from
3d0627a to
40a7708
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/backend/src/gitlab.ts (1)
183-219:⚠️ Potential issue | 🟠 MajorSize filtering won't apply to individually specified projects.
The
config.projectspath usesapi.Projects.show(project)which does not includestatistics: truefromprojectListOptions. As a result,getProjectSizeByteswill returnundefinedfor these projects, and size-based exclusion will be silently skipped. If a user configures bothprojectsandexclude.size, they may not get the expected behavior.Consider either:
- Passing
statistics: truetoProjects.show()whenexclude.sizeis configured, or- Documenting that size filtering only applies to projects discovered via
all,groups, orusers.Proposed fix
const { durationMs, data } = await measure(async () => { - const fetchFn = () => api.Projects.show(project); + const fetchFn = () => api.Projects.show(project, { + ...(config.exclude?.size ? { statistics: true } : {}), + }); return fetchWithRetry(fetchFn, `project ${project}`, logger); });
🤖 Fix all issues with AI agents
In `@packages/schemas/src/v3/connection.type.ts`:
- Around line 138-141: Update the union type for minAccessLevel in the
Connection type: remove the invalid value 0 and add 15 (Planner) so the field
reads minAccessLevel?: 5 | 10 | 15 | 20 | 30 | 40 | 50; — locate the
minAccessLevel property in connection.type.ts and adjust the allowed numeric
literals accordingly to match GitLab's API.
🧹 Nitpick comments (4)
packages/schemas/src/v3/gitlab.schema.ts (1)
167-180: Consider enforcing non-negative size bounds.Adding a minimum of 0 for
min/maxprevents negative sizes in config. Since this file is auto-generated, update the schema source/generator accordingly.♻️ Suggested schema tweak
"min": { "type": "integer", + "minimum": 0, "description": "Minimum project size (in bytes) to sync (inclusive). Projects smaller than this will be excluded." }, "max": { "type": "integer", + "minimum": 0, "description": "Maximum project size (in bytes) to sync (inclusive). Projects larger than this will be excluded." }schemas/v3/gitlab.json (1)
139-153: Schema definition forexclude.sizelooks good.
additionalProperties: falseis correctly set. Consider addingminimum: 0to theminandmaxproperties to prevent nonsensical negative byte values at the schema validation level, though this is a minor hardening.packages/backend/src/gitlab.ts (2)
279-292: Silent skip when statistics are missing could be surprising.When
exclude.sizeis configured butgetProjectSizeBytesreturnsundefined(e.g., statistics not available), the project passes through without any size check or warning. Consider logging a debug/warning message when statistics are expected but missing, so operators can diagnose why size filtering isn't working for specific projects.Proposed enhancement
if (exclude?.size) { const projectSizeBytes = getProjectSizeBytes(project); if (projectSizeBytes !== undefined) { if (exclude.size.min !== undefined && projectSizeBytes < exclude.size.min) { reason = `project size (${projectSizeBytes}) is less than \`exclude.size.min\` (${exclude.size.min})`; return true; } if (exclude.size.max !== undefined && projectSizeBytes > exclude.size.max) { reason = `project size (${projectSizeBytes}) is greater than \`exclude.size.max\` (${exclude.size.max})`; return true; } + } else { + logger.debug(`No statistics available for project ${projectName}, skipping size-based exclusion check.`); } }
325-356:getProjectSizeBytessilently falls back fromstorage_sizetorepository_size— these metrics differ significantly.
storage_sizeincludes the entire project (repo, LFS, packages, artifacts, etc.), whilerepository_sizeis just the git repository. Falling through from one to the other without any indication could lead to inconsistent filtering behavior across projects. The field name priority should be documented in the schema description or in a code comment explaining the preference order.Also, the dual snake_case/camelCase handling is a good defensive measure against
@gitbeaker/restresponse serialization differences.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/backend/src/gitlab.ts`:
- Around line 64-73: projectListOptions only sets statistics: true when
config.exclude?.size is present, but the Projects.show() call (used to fetch
individually listed projects) doesn't pass that option, so project.statistics
stays undefined and getProjectSizeBytes() can't filter by size; update the call
to Projects.show(...) to pass the same options object (or at least { statistics:
true } when config.exclude?.size) as the second argument so that
project.statistics is populated for size filtering—use the existing
projectListOptions or a conditional { statistics: true } when calling
Projects.show in the code path that loads individual projects.
🧹 Nitpick comments (2)
packages/backend/src/gitlab.ts (1)
279-292: Silent skip when statistics are unavailable — consider logging a warning.When
exclude.sizeis configured butgetProjectSizeBytesreturnsundefined(e.g., statistics not available for the project), the size filter is silently bypassed. A debug/warn log here would help users troubleshoot why certain projects aren't being filtered.Proposed improvement
if (exclude?.size) { const projectSizeBytes = getProjectSizeBytes(project); if (projectSizeBytes !== undefined) { if (exclude.size.min !== undefined && projectSizeBytes < exclude.size.min) { reason = `project size (${projectSizeBytes}) is less than \`exclude.size.min\` (${exclude.size.min})`; return true; } if (exclude.size.max !== undefined && projectSizeBytes > exclude.size.max) { reason = `project size (${projectSizeBytes}) is greater than \`exclude.size.max\` (${exclude.size.max})`; return true; } + } else { + logger.warn(`Size exclusion configured but no statistics available for project ${projectName}. Skipping size filter.`); } }schemas/v3/gitlab.json (1)
138-153: Consider clarifying which GitLab size field is used.The description says "GitLab statistics size fields" but doesn't specify which field. The backend implementation (
getProjectSizeBytes) preferentially usesstorage_size(which includes LFS, artifacts, etc.) and falls back torepository_size. Documenting this in the schema description would help users set appropriate thresholds.Suggested description improvement
- "description": "Exclude projects based on GitLab statistics size fields (in bytes).", + "description": "Exclude projects based on GitLab statistics size fields (in bytes). Uses `storage_size` when available (includes LFS, artifacts, etc.), falling back to `repository_size`.",
Support excluding GitLab projects by statistics-backed size bounds and pass through minAccessLevel for project listing with AccessLevel-aligned typing.
WilliamZhang9
force-pushed
the
u/williamzhang/issue_564_add_repo_size_exclude_option_for_gitlab
branch
from
525570a to
a65ffd8
Compare
|
It looks only users with the Reporter, Developer, Maintainer, or Owner roles can retrieve a GitLab project’s size. Do we need to use 
|
1 participant
Problem
#564
Solution
Support excluding GitLab projects by statistics-backed size bounds and pass through
minAccessLevelfor project, and align it with the GitLab access level.Summary by CodeRabbit
New Features
Tests