search

fedora-infra / mirrormanager2

Rewrite of the MirrorManager application in Flask and SQLAlchemy
https://mirrormanager.fedoraproject.org
GNU General Public License v2.0
65 stars 49 forks source link

Remove <0.80 requirement on IPy #193

Closed puiterwijk closed 7 years ago

puiterwijk commented 7 years ago

Signed-off-by: Patrick Uiterwijk puiterwijk@redhat.com

pypingou commented 7 years ago

Because the issue is fixed?

puiterwijk commented 7 years ago

Because I have not been able to see any reason why it was added (requirements.txt doesn't list the version requirement, so pip would happily install the latest) and this has not been changed since its introduction two years ago in the spec file.

pypingou commented 7 years ago

In theory I have nothing against, but I'd rather finding back why this was added before actually merging this PR.

bowlofeggs commented 7 years ago

The IPy 0.80 release notes don't say anything that sounds scary to me:

https://github.com/autocracy/python-ipy/blob/master/ChangeLog

I'm comfortable with dropping the requirement, given the release notes. I agree with @pypingou that it would be helpful to know why that requirement was put in place, but it's also possible that that information has been lost.

adrianreber commented 7 years ago

I cannot load the pkl on Fedora with python-IPy-0.81-16.fc25.noarch:

TypeError: ('init() takes at least 2 arguments (1 given)', <class 'IPy.IP'>, ())

Same code works on CentOS7 with python-IPy-0.75-6.el7.noarch

Not really sure where this comes from.

pypingou commented 7 years ago

It is my understanding that this has now been done on the servers of the Fedora Infra, and afaict it all works.

So I guess I should lift my suspicions and say 👍 to this PR :)

adrianreber commented 7 years ago

The main problem seems to be that it is not possible to mix versions. A pkl generated with an older version does not work in a newer version. But yes, let's merge this.