Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

chore: Add support for python 3.11 #1115

Merged
merged 14 commits into from
Nov 4, 2022

Conversation

laurentS
Copy link
Contributor

@laurentS laurentS commented Oct 26, 2022

This just adds a few lines to run CI tests on the recently released python 3.11.

Substantial speedups are expected, and as the taps and targets are run inside their own virtual envs, it should be pretty easy to upgrade them even if the main app is running on an earlier version of python.


📚 Documentation preview 📚: https://meltano-sdk--1115.org.readthedocs.build/en/1115/

@laurentS laurentS requested review from edgarrmondragon and a team as code owners October 26, 2022 13:19
@edgarrmondragon
Copy link
Collaborator

edgarrmondragon commented Oct 26, 2022

Hey @laurentS!

You'll need to add 3.11 to the known version in noxfile.py:

- python_versions = ["3.10", "3.9", "3.8", "3.7"]
+ python_versions = ["3.11", "3.10", "3.9", "3.8", "3.7"]

Even then, I'm not sure it'll work because of our (dev) dependency on Numpy. The older version we pull may not have wheels for 3.11 or not even support it at all, and we can't bump Numpy until we drop support for 3.7 🤷‍♂️

@laurentS
Copy link
Contributor Author

Ok, it looks liks numpy is brought in by pyarrow which itself is working on supporting 3.11 (see this issue). Looks like they're still supporting python 3.7, so chances are they'll fix the problem for us :)
I'll keep an eye on it, and come back here when there's a new release over there. I have a feeling it won't take long.

@aaronsteers
Copy link
Contributor

aaronsteers commented Oct 27, 2022

Agreed we can probably wait this out if pyarrow/numpy updates are coming soon.

Since pyarrow is only a dev dependency, we can optionally remove it at the cost of disabling/removing the target-parquet sample implementation.

To my knowledge (and with a quick search to confirm), this is the only place where the pyarrow dependency is used as of now:

import pyarrow as pa
import pyarrow.parquet as pq

@laurentS
Copy link
Contributor Author

I'm wondering if these dependencies that are only used for samples would be better managed as extras, like is done for docs?

@edgarrmondragon
Copy link
Collaborator

I'm wondering if these dependencies that are only used for samples would be better managed as extras, like is done for docs?

@laurentS Poetry would still resolve those dependencies along with the rest even if they're extras. The samples would need their own separate dependency trees/pyproject.toml.

@codecov
Copy link

codecov bot commented Oct 28, 2022

Codecov Report

Merging #1115 (30b6111) into main (f255dae) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #1115   +/-   ##
=======================================
  Coverage   83.49%   83.49%           
=======================================
  Files          42       42           
  Lines        3854     3854           
  Branches      657      657           
=======================================
  Hits         3218     3218           
  Misses        472      472           
  Partials      164      164           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@laurentS laurentS changed the title Run tests on python 3.11 chore: add support for python 3.11 Oct 28, 2022
@laurentS
Copy link
Contributor Author

I'm seeing 2 packages that cause problems with python 3.11:

  • pyarrow, used only for the target-parquet sample code. I feel like there's value in having tested code samples that are known to run, but also wonder if it's worth blocking the entire tap+target ecosystem in a situation like this. In order to progress on this PR, I've marked the test as skipped for python 3.11, so as to avoid dropping checks that were in place. Once pyarrow releases a new version, we can re-enable the test and imports for all versions of python.
  • viztracer Can I suggest removing it entirely from the pyproject.toml file? This is a debug tool, I think it's enough to have the docs mention how to install it, and nothing else. Right now, it's being installed multiple times on each CI run, without being used. It costs CI time 🕐 and carbon footprint ☁️

At this point, all checks are green, with a bit of a caveat for 3.11. I had to adjust dependency specifications for older pythons, as newer releases came out recently that broke tests.

@edgarrmondragon
Copy link
Collaborator

edgarrmondragon commented Oct 28, 2022

  • viztracer Can I suggest removing it entirely from the pyproject.toml file? This is a debug tool, I think it's enough to have the docs mention how to install it, and nothing else. Right now, it's being installed multiple times on each CI run, without being used. It costs CI time 🕐 and carbon footprint ☁️

@laurentS Agreed 100%. See #1124.

@edgarrmondragon
Copy link
Collaborator

  • pyarrow, used only for the target-parquet sample code. I feel like there's value in having tested code samples that are known to run, but also wonder if it's worth blocking the entire tap+target ecosystem in a situation like this. In order to progress on this PR, I've marked the test as skipped for python 3.11, so as to avoid dropping checks that were in place. Once pyarrow releases a new version, we can re-enable the test and imports for all versions of python.

@laurentS Sounds good. Skipping the test on 3.11 is a good approach 👍

pyproject.toml Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
Copy link
Contributor

@aaronsteers aaronsteers left a comment

Choose a reason for hiding this comment

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

With the latest updates, this is looking good to me. 👍

Thanks very much.

@edgarrmondragon
Copy link
Collaborator

Alright @laurentS, this LGTM! Thanks so much for making all the necessary updates in tests and dependencies.

@edgarrmondragon edgarrmondragon changed the title chore: add support for python 3.11 chore: Add support for python 3.11 Nov 4, 2022
@edgarrmondragon edgarrmondragon merged commit 75965be into meltano:main Nov 4, 2022
@laurentS laurentS deleted the add-support-for-python-3-11 branch November 7, 2022 10:44
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants