rpm-software-management/createrepo_c

Undocumented change in python API

lubomir opened this issue · 6 comments

lubomir commented

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 commented

Thank you for the report. It sounds critical.

kontura commented

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

lubomir commented

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 commented

@kontura This can be closed now yes?

  • 👍1
dralley commented

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 commented

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.