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

unfix SQLAlchemy version, to not conflict with latest CKAN #30

Merged
merged 1 commit into from
Oct 8, 2015

Conversation

deniszgonjanin
Copy link
Contributor

No description provided.

@florianm
Copy link

florianm commented Oct 1, 2015

+1 (watching this PR with some urgency from my end) :-)

@amercader
Copy link
Member

Can you elaborate a bit on this is an issue? Is because of a problem with a particular SQLAlchemy version or because it conflicts with the CKAN one?

@deniszgonjanin
Copy link
Contributor Author

Yes, it's just a version conflict. ckan-service-provider doesn't need to be affixed to sqlalchemy 0.7.8, so it's better to let the version float. Otherwise it conflicts with newest ckan, as well as newer versions of spatial I think.

@florianm
Copy link

florianm commented Oct 8, 2015

Can I raise the conflict of requests==2.5.0 as well? Many newer extensions are pinned to requests==2.7.0. E.g. ckan-galleries breaks on this. AFAIK requests==2.7.0 does some ssh handling differently, but nothing that impacts CKAN.

@amercader amercader merged commit cbffdf7 into ckan:master Oct 8, 2015
@amercader
Copy link
Member

@florianm AFAICT ckan core has requests==2.3.0 pinned down so that is a separate issue I think

@florianm
Copy link

florianm commented Oct 8, 2015

Fair enough, just out of interest, would unpinning requests introduce risk?
Assuming that csp will always be installed next to a ckan with a moderately
conservative requests version pinned.
On 08/10/2015 9:12 pm, "Adrià Mercader" [email protected] wrote:

@florianm https://github.com/florianm AFAICT ckan core has
requests==2.3.0 pinned down so that is a separate issue I think


Reply to this email directly or view it on GitHub
#30 (comment)
.

@davidread
Copy link
Contributor

An easy change is to move these requirements out of setup.py into a requirements.txt, and then:

  • newbies can have a pinned version, that avoids problems when new versions come out
  • people that know what they are doing can mess with the version much more easily

@florianm
Copy link

florianm commented Oct 9, 2015

This would also make the requirements turn up in http://rshiny.yes-we-ckan.org/ckan-pip-collisions/ which simple-mindedly only collects dependencies from requirements* files.

Streamlining and overriding requirements of ckan and all extensions would also be easier with a simple

grep -rl --include="*requirements*" 'requests' . | xargs sed -i 's/^.*requests.*$/requests==2.7.0/g'

@amercader
Copy link
Member

+1 on using requirements.txt, which is the pattern we use everywhere else

@amercader
Copy link
Member

But thinking about it that might affect the install process as ckan-service-provider is listed as a package from pypi on the requirements.txt of datapusher...

@florianm
Copy link

florianm commented Oct 9, 2015

From my non-core-developer end, datacats is working again, I'm very happy
to not touch things that currently work :-)
ckan/ckanext-spatial#117 is far more important for me right now :-D
On 09/10/2015 5:43 pm, "Adrià Mercader" [email protected] wrote:

But thinking about it that might affect the install process as
ckan-service-provider is listed as a package from pypi on the
requirements.txt of datapusher...


Reply to this email directly or view it on GitHub
#30 (comment)
.

@florianm
Copy link

Update, ckan core is now pinned to requests==2.7.0
On 09/10/2015 6:22 pm, "Florian May" [email protected]
wrote:

From my non-core-developer end, datacats is working again, I'm very happy
to not touch things that currently work :-)
ckan/ckanext-spatial#117 is far more important for me right now :-D
On 09/10/2015 5:43 pm, "Adrià Mercader" [email protected] wrote:

But thinking about it that might affect the install process as
ckan-service-provider is listed as a package from pypi on the
requirements.txt of datapusher...


Reply to this email directly or view it on GitHub
#30 (comment)
.

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.

4 participants