Skip to content

Make connectors work with python 3.10 and 3.11#2734

Merged
artem-shelkovnikov merged 70 commits intomainfrom
artem/relax-python-requirements
Aug 14, 2024
Merged

Make connectors work with python 3.10 and 3.11#2734
artem-shelkovnikov merged 70 commits intomainfrom
artem/relax-python-requirements

Conversation

@artem-shelkovnikov
Copy link
Member

@artem-shelkovnikov artem-shelkovnikov commented Aug 5, 2024

Part of #2672

This PR makes connectors work with 3.10 and 3.11 version of Python. 3.12 is not supported yet because of some deprecations for datetime package that we need to handle: https://blog.miguelgrinberg.com/post/it-s-time-for-a-change-datetime-utcnow-is-now-deprecated. Once it's handled, adding 3.12 to matrix is gonna be trivial.

CI image was updated to make it work, here's the CI image script: https://github.com/elastic/ci-agent-images/blob/main/vm-images/enterprise-search/scripts/connectors-python/install-deps.sh

Link to nightlies using this branch: https://buildkite.com/elastic/connectors-nightly/builds/2402

Checklists

Pre-Review Checklist

  • this PR does NOT contain credentials of any kind, such as API keys or username/passwords (double check config.yml.example)
  • this PR has a meaningful title
  • this PR links to all relevant github issues that it fixes or partially addresses
  • if there is no GH issue, please create it. Each PR should have a link to an issue
  • this PR has a thorough description
  • Covered the changes with automated tests
  • Tested the changes locally
  • Added a label for each target release version (example: v7.13.2, v7.14.0, v8.0.0)
  • Considered corresponding documentation changes

Release Note

Now it's possible to run connectors with Python 3.11 as well as Python 3.10

@artem-shelkovnikov artem-shelkovnikov marked this pull request as ready for review August 12, 2024 12:23
@artem-shelkovnikov artem-shelkovnikov requested a review from a team August 12, 2024 12:23
@seanstory
Copy link
Member

Holding off on review while the build is red, but holler when this is ready 🤞

@artem-shelkovnikov
Copy link
Member Author

Holding off on review while the build is red, but holler when this is ready 🤞

It's just a random blip from CI not being able to start Elasticsearch, feel free to review :)

Copy link
Member

@seanstory seanstory left a comment

Choose a reason for hiding this comment

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

few comments/questions, but looks pretty straightforward, honestly. Nice work!

I kicked off a nightly, just for a double check: https://buildkite.com/elastic/connectors-nightly/builds/2404

Copy link
Member

@seanstory seanstory left a comment

Choose a reason for hiding this comment

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

also, realized that 3.12 had slipped through in a few places

Copy link
Member

@seanstory seanstory left a comment

Choose a reason for hiding this comment

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

:shipit:

@artem-shelkovnikov artem-shelkovnikov merged commit 5c9865a into main Aug 14, 2024
@artem-shelkovnikov artem-shelkovnikov deleted the artem/relax-python-requirements branch August 14, 2024 12:31
@github-actions
Copy link

💔 Failed to create backport PR(s)

The backport operation could not be completed due to the following error:
There are no branches to backport to. Aborting.

The backport PRs will be merged automatically after passing CI.

To backport manually run:
backport --pr 2734 --autoMerge --autoMergeMethod squash

@@ -1,6 +1,6 @@
.PHONY: test lint autoformat run ftest install release docker-build docker-run docker-push

PYTHON=python3.10
Copy link
Member

Choose a reason for hiding this comment

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

this change requires your setup to have an alias for python, in my machine I didn't had that and to make it work I have to do PYTHON=python3 make install

Is this an issue ? If no do we want to mention it in somewhere?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants