Skip to content

sync: add max_size option#89

Merged
qa-cea merged 5 commits intocea-hpc:masterfrom
valeriyoann:sync_max_size
Apr 22, 2026
Merged

sync: add max_size option#89
qa-cea merged 5 commits intocea-hpc:masterfrom
valeriyoann:sync_max_size

Conversation

@valeriyoann
Copy link
Copy Markdown
Contributor

Add a "max_size" option to the sync command, to prevent the download of large files.

@valeriyoann valeriyoann requested review from qa-cea and rezib April 3, 2026 08:35
Copy link
Copy Markdown
Collaborator

@rezib rezib left a comment

Choose a reason for hiding this comment

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

I suggest adding a unit test of download_file() utils to cover the new case, with a mocked urlopen() result.

I also suggest 3 easy fixes for new pylint errors introduced.

Pylint also complains about the number of args on many signatures in sync.py. I see that config is not used that much, only for proxy as far as I've seen. Maybe we should consider in the future a small refactoring to provide a SyncOpts object (or similar) to synchronizers with proxy settings and max size, and potentially extensible with other parameters in the future. But not for today.

Comment thread lib/rift/utils.py Outdated
This patch adds a max_size option for the sync command, to limit the
download of RPMs too large.
Signed-off-by: VALERI Yoann <yoann.valeri@cea.fr>
Signed-off-by: VALERI Yoann <yoann.valeri@cea.fr>
Signed-off-by: VALERI Yoann <yoann.valeri@cea.fr>
Signed-off-by: VALERI Yoann <yoann.valeri@cea.fr>
Comment thread lib/rift/utils.py
@valeriyoann valeriyoann force-pushed the sync_max_size branch 6 times, most recently from b0fdd1c to 2cc78bd Compare April 21, 2026 13:23
@qa-cea qa-cea self-requested a review April 21, 2026 14:28
Comment thread tests/utils.py Outdated
Signed-off-by: VALERI Yoann <yoann.valeri@cea.fr>
@qa-cea qa-cea merged commit 15ead62 into cea-hpc:master Apr 22, 2026
4 of 5 checks passed
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.

3 participants