-
Notifications
You must be signed in to change notification settings - Fork 91
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
repo: Add option to download all repository metadata #1879
Conversation
Needed for `dnf5 reposync --download-metadata` command.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would make sense to me that when all
is set it will not only download all metadata types but it will also load and use them. Basically there is a few more places that check what does optional_metadata_types
config option contain and they should take all
into account.
I think it is important for API users.
Plus it would be good to document the new type in the optional_metadata_types
config option docs.
const std::set<std::string> OPTIONAL_METADATA_TYPES{ | ||
METADATA_TYPE_COMPS, METADATA_TYPE_FILELISTS, METADATA_TYPE_OTHER, METADATA_TYPE_PRESTO, METADATA_TYPE_UPDATEINFO}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like this set
is meant to represent all the optional metadata types but its just the ones known to libdnf5, so it is different to the new all
.
(Also I think both of its uses are questionable. At least the changelog
plugin doesn't need to download/load all of these metadata types, I think it needs just METADATA_TYPE_OTHER
.)
This is not that related so I don't believe it should block this PR but some comment that describes the difference between all
and OPTIONAL_METADATA_TYPES
would be nice.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, I'm thinking about getting rid of OPTIONAL_METADATA_TYPES - changelog definitely doesn't need it and system repo loading should be fine with METADATA_TYPE_ALL.
I can remove it's usage, but since it is on public API I can only mark it deprecated. But the set itself needs to stay for a while.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Regarding the system repo I am not sure what its supposed to mean. I think the system repo can only load comps
: https://github.com/rpm-software-management/dnf5/blob/main/libdnf5/repo/solv_repo.cpp#L286-L318
I don't think the RepoDownloader::get_optional_metadata()
is ever used for system repo.
I haven't really investigated it but in my mind RepoDownloader
doesn't seem applicable to system repo.
Changelog does not need to download all metadata types from repository. Only "other" metadata type is needed.
@kontura thanks for the review! |
Needed for
dnf5 reposync --download-metadata
command.