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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade RDFLib requirement #213

Merged
merged 16 commits into from May 25, 2022
Merged

Upgrade RDFLib requirement #213

merged 16 commits into from May 25, 2022

Conversation

amercader
Copy link
Member

The current version of RDFlib won't work on Python 3.9 so we need to upgrade to RDFlib 6, which is not compatible with Python 2. The JSON-LD requirement has been integrated into the main rdflib package.

This adapts the tests so they can run across all RDFLib / Python / CKAN version 馃

On RDFlib 6 the default format is turtle, not xml
For some reason this stopped working on Python 3.9 or latest responses
version
RDFlib 6 binds the Schema.org namespace using https://schema.org. Our
profiles bind the http variant, which gets registered as `schema1`. This
change ensures that we are replacing the existing bind with the http
version to maintain compatibility with the current serializations.
The current version of RDFlib won't work on Python 3.9 so we need to
upgrade to RDFlib 6, which is not compatible with Python 2.
The JSON-LD requirement ahs been integrated into the main rdflib package
Because of ckan/ckan#6841 the host name can not be overriden in the
tests that use a request context
@amercader
Copy link
Member Author

I finally managed to get all versions green 馃檧
@seitenbau-govdata (and others) I'm fairly confident that the RDFlib upgrade won't break existing serializations and parsers but I'd be supergrateful if this PR could be tested in a real life instance with real metadata.

@seitenbau-govdata
Copy link
Member

@amercader Sure, we can do that. We can test it in the environments CKAN 2.8 under Python 2.7 and CKAN 2.9 under Python 3.6.

@seitenbau-govdata
Copy link
Member

seitenbau-govdata commented May 10, 2022

@amercader Sorry, our environment (Ubuntu 18.04) is currently locked to Python 3.6. Unfortunately RDFlib 6 requires Python 3.7+, which means we cannot test this at the moment. Ubuntu 18.04 is supported until April 2023. So the only possible versions are rdflib in 5.0.0 and rdflib-jsonld in 0.6.2. But I don't know if the versions support Python 3.8+.

@amercader
Copy link
Member Author

@seitenbau-govdata It's a tricky balance. I know that Ubuntu 18.04 might be supported for a while, but Python 3.6 is no longer supported and doesn't receive security updates any more. I'm keen on maximizing support for this extension but also using recent versions to minimize issues like #205.

If I understand correctly, users running on Python 3.6 can still use the extension if they install the requirements-py2.txt ones right? Obviously we need to document that on the README (and maybe rename the file to requirements-py2-py36.txt) but do you think this would be reasonable?

@seitenbau-govdata
Copy link
Member

@amercader True, it's really tricky. Yes, the requirements in requirements-py2.txt still working under Python 3.6. I think this is a solution we can go with. Adding a hint in the documentation sounds good to me. Probably renaming the requirements file to requirements-py2-py36.txtmakes it even clearer than just documenting it on the README.

Currently we are testing the branch with the dependencies rdflib==5.0.0 and rdflib-jsonld==0.5.0 under CKAN 2.9 (Python 3.6) and CKAN 2.8 (Python 2.7). It wouldn't hurt to update the dependencies at the same time.

@seitenbau-govdata
Copy link
Member

@amercader We have successfully tested the branch with the following environments

  • CKAN 2.9.5 / Python 3.6
  • CKAN 2.9.5 / Python 2.7
  • CKAN 2.8.7 / Python 2.7

and the requirements rdflib==5.0.0 and rdflib-jsonld==0.5.0 in requirements-py2.txt.

But with Python 2.7 we suggest a downgrade in the dev-rquirements-py2.txt to responses==0.10.15, because there exists a version conflict with urllib3 between responses >= 0.10.15 and CKAN core Python 2 requirements.

ERROR: pip's legacy dependency resolver does not consider dependency conflicts when selecting packages. This behaviour is the source of the following dependency conflicts.
Successfully installed pytest-ckan-0.0.12 responses-0.17.0 urllib3-1.26.9
requests 2.22.0 requires urllib3!=1.25.0,!=1.25.1,<1.26,>=1.21.1, but you'll have urllib3 1.26.9 which is incompatible.

https://github.com/ckan/ckanext-dcat/runs/6289349808?check_suite_focus=true#step:5:100

This become an issue if the requirement ckanext-harvest is installed before ckanext-dcat. In the GitHub actions the ckanext-harvest is installed after ckanext-dcat so the version is replaced by a CKAN core compatible version.

Requirement already satisfied: idna<2.9,>=2.5 in /usr/lib/python2.7/site-packages (from requests>=2.11.1->-r ckanext-harvest/pip-requirements.txt (line 5)) (2.8)
Collecting urllib3!=1.25.0,!=1.25.1,<1.26,>=1.21.1
  Downloading urllib3-1.25.11-py2.py3-none-any.whl (127 kB)
Installing collected packages: pika, pyOpenSSL, urllib3
  Attempting uninstall: urllib3
    Found existing installation: urllib3 1.26.9
    Uninstalling urllib3-1.26.9:
      Successfully uninstalled urllib3-1.26.9
Successfully installed pika-1.2.1 pyOpenSSL-18.0.0 urllib3-1.25.11

https://github.com/ckan/ckanext-dcat/runs/6289349808?check_suite_focus=true#step:6:63

@seitenbau-govdata
Copy link
Member

@amercader And we need a rebase to the current master and adding the self._add_responses_solr_passthru() to the new tests

  • test_harvest_default_file_size
  • test_harvest_config_file_size

@seitenbau-govdata
Copy link
Member

@amercader 馃憤 LGTM, thanks!

@amercader
Copy link
Member Author

Thanks @seitenbau-govdata ! I've incorporated all your suggested changes, I think that gives a good support across versions, I'll release a new version with this and your other recent changes.

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.

None yet

2 participants