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

Undocumented change in python API #360

Closed
lubomir opened this issue Apr 25, 2023 · 6 comments
Closed

Undocumented change in python API #360

lubomir opened this issue Apr 25, 2023 · 6 comments
Assignees

Comments

@lubomir
Copy link

lubomir commented Apr 25, 2023

Version 0.21.1 introduced changes in the python API for accessing files in a particular package. The tuple describing a file entry grew from 3 items to 4. I have not found any documentation on what the fourth item is, and there is no cr.FILE_ENTRY_ constant to access it.

This change breaks backwards compatibility if anyone is using code like:

for type_, path, name in pkg.files:
    ....

Here's a small piece of code to replicate the change:

import createrepo_c as cr
md = cr.Metadata()
md.locate_and_load_xml("https://kojipkgs.fedoraproject.org/compose/rawhide/latest-Fedora-Rawhide/compose/Everything/x86_64/os/")
print(len(md.get(md.keys()[0]).files[0]))
$ rpm -qa | grep createrepo_c
createrepo_c-libs-0.20.1-1.fc36.x86_64
createrepo_c-0.20.1-1.fc36.x86_64
python3-createrepo_c-0.20.1-1.fc36.x86_64
$ python replicator.py 
3
$ sudo dnf update createrepo_c 
...
$ rpm -qa | grep createrepo_c
createrepo_c-libs-0.21.1-1.fc36.x86_64
createrepo_c-0.21.1-1.fc36.x86_64
python3-createrepo_c-0.21.1-1.fc36.x86_64
$ python replicator.py 
4
@j-mracek
Copy link
Member

Thank you for the report. It sounds critical.

@kontura
Copy link
Contributor

kontura commented Apr 26, 2023

I made a PR #362 to fix this.
Can you please check if everything works well with it for your use cases?

@lubomir
Copy link
Author

lubomir commented Apr 26, 2023

That looks good to me. I have changed the code where I found this to use the constants rather than unpacking, so it works with both old and new version. Now the fourth item is documented, so I'm happy.

@dralley
Copy link
Contributor

dralley commented Jul 31, 2023

@kontura This can be closed now yes?

@dralley
Copy link
Contributor

dralley commented Jul 31, 2023

Should a new issue be filed for my comment on the PR?

However to avoid this in the future maybe it makes sense to return a class/dataclass rather than a tuple, so that data can be added without breaking API. Maybe for 1.0? (not backportable obviously)

kontura added a commit to kontura/createrepo_c that referenced this issue Aug 8, 2023
@kontura
Copy link
Contributor

kontura commented Aug 8, 2023

Should a new issue be filed for my comment on the PR?

However to avoid this in the future maybe it makes sense to return a class/dataclass rather than a tuple, so that data can be added without breaking API. Maybe for 1.0? (not backportable obviously)

There is more tuples like this: https://github.com/rpm-software-management/createrepo_c/blob/master/src/python/createrepo_c/__init__.py#L100-L118 plus undocumented DistroTag.

I am not sure.
The change would be quite disruptive but maybe the filelists will be extended again in the future.

Perhaps we could convert it together with the addition of new elements (if that ever happens).
I think this would be better tracked in code rather than a new issue.

@kontura kontura closed this as completed Aug 8, 2023
kontura added a commit that referenced this issue Aug 9, 2023
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

No branches or pull requests

4 participants