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

Compatibility with Pydantic >= 2 through pydantic.v1 #1663

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

SarahG-579462
Copy link

Overview

Recently, pygeoapi's pydantic was downgraded from 2.0-compatible to pydantic < 1. This removed compatibility with two of our libraries, xscen and xclim, for climate data processing, due to intake-esm's current requirement on pydantic > 2

Because we require these in the use of our pygeoapi process plugins, this change forced us to use either very outdated versions of xscen (and thus xarray, pandas, xclim, etc.), or a very outdated version of pygeoapi.

Thankfully, a simple fix is available by the use of pydantic 2's backwards-compatible pydantic.v1 module.

It is not entirely clear to me why pydantic was downgraded, the best explanation I can see in the discussion was due to "versionitis". I was, however, able to run all tests with the latest version of pydantic installed in this fashion. The way this PR is written, if pydantic >= 2 is not available on the operating system in question, everything will continue as-is.

Related Issue / discussion

Pydantic issues:
#1573
#1341
#1584

Additional information

Dependency policy (RFC2)

  • I have ensured that this PR meets RFC2 requirements

(It follows RFC2 as far as I understand, as it allows backwards compatibility with pydantic < 2)

Updates to public demo

No breaking changes shuold ensue as a result of this PR, since it is backwards compatible. I am not entirely sure how to test this demo server, help would be appreciated here.

Contributions and licensing

(as per https://github.com/geopython/pygeoapi/blob/master/CONTRIBUTING.md#contributions-and-licensing)

  • I'd like to contribute [feature X|bugfix Y|docs|something else] to pygeoapi. I confirm that my contributions to pygeoapi will be compatible with the pygeoapi license guidelines at the time of contribution
  • I have already previously agreed to the pygeoapi Contributions and Licensing Guidelines

@SarahG-579462
Copy link
Author

Note: I had to change one of the github actions, since docker images do not allow capital letters, and the repository for our organisation is "Ouranosinc". Feel free to discard this change, if needed.

@@ -21,6 +21,10 @@ jobs:
run:
working-directory: .
steps:
- id: string
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the nature of this update? Should this be in another PR?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is related to the comment above. because I was working on a fork which has a capital letter in the repo name, it was failing to build the docker image in this github action. I don't think it's a great idea for this particular case, because it relies on a third party github action for security vulnerability checks, so I'll yeet it. 🙂

@@ -1,2 +1,2 @@
Django
pydantic<2.0
pydantic
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure how this landed in requirements-django.txt, but given that pydantic is in requirements.txt I would actually remove this requirement in requirements-django.txt.

@@ -4,7 +4,7 @@ filelock
Flask
jinja2
jsonschema
pydantic<2.0
pydantic
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a timeline for pydantic 2 on Ubuntu 22.04? We need to consider this for this PR (note that our next release is scheduled for next month).

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you saying that without pydantic<2.0 here, it will override the system pydantic version, if someone is building without a virtual env, and that is an issue?

@tomkralidis
Copy link
Member

To add, it would be valuable to have pygeoapi.models.pydantic that covers the try/except in one place and have the rest of code import from pygeoapi.models.pydantic

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