Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

osbuild: add org.osbuild.librepo source wrapper #1141

Merged
merged 1 commit into from
Jan 15, 2025

Conversation

mvo5
Copy link
Contributor

@mvo5 mvo5 commented Jan 14, 2025

This commit adds a wrapper for the org.osbuild.librepo source
to download RPMs.

Note that this does not allow customizing the librepo options
MaxParallels, FastestMirror yet. We need to brainstorm this
a bit. The reason is that GenSources() does not currently allow
much control so our (library) users cannot customize it easily.

Split out/cleaned up version of #1132

pkg/osbuild/librepo_source.go Outdated Show resolved Hide resolved
pkg/osbuild/librepo_source.go Outdated Show resolved Hide resolved
pkg/osbuild/librepo_source.go Outdated Show resolved Hide resolved
Comment on lines 11 to 12
Items map[string]*librepoSourceItem `json:"items"`
Options *librepoSourceOptions `json:"options"`
Copy link
Member

Choose a reason for hiding this comment

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

I'd prefer we make the types exported/public so that one can, if they want, create a LibrepoSource{} without functions or helpers. It's not the preferred way to do it, but I think pkg/osbuild/ should support that workflow generally.

Copy link
Contributor Author

@mvo5 mvo5 Jan 14, 2025

Choose a reason for hiding this comment

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

I can change them back to exported (there were in my first version), I changed them because it easy to create non-working osbuild.LibrepoSources when doing this by hand, the repos and package IDs must match etc so this is meant as a hint "use-the-helper". But fine to change them as the rest of the package exports all types (I guess consistency here is more important than my whims)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I changed it back to export everything. I would love to reduce our API surface though longer-term but not a discussion we need for this PR :)

@mvo5 mvo5 force-pushed the osbuild/add-librepo-wrapper branch 2 times, most recently from 03e8b44 to a8bfc92 Compare January 14, 2025 19:29
@mvo5 mvo5 requested a review from achilleas-k January 14, 2025 19:29
This commit adds a wrapper for the org.osbuild.librepo source
to download RPMs.
Copy link
Member

@ondrejbudai ondrejbudai left a comment

Choose a reason for hiding this comment

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

Thanks! ❤️

@mvo5 mvo5 enabled auto-merge January 15, 2025 10:24
@mvo5 mvo5 added this pull request to the merge queue Jan 15, 2025
Merged via the queue into osbuild:main with commit e405107 Jan 15, 2025
18 checks passed
@mvo5 mvo5 deleted the osbuild/add-librepo-wrapper branch January 15, 2025 11:05
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