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

Add support for Python 3.11 #639

Merged
merged 9 commits into from
Oct 26, 2022
Merged

Add support for Python 3.11 #639

merged 9 commits into from
Oct 26, 2022

Conversation

parasj
Copy link
Contributor

@parasj parasj commented Oct 25, 2022

Changes:

  • Update Dockerfile to Python 3.11
  • Test pytest on CI with Python 3.11
  • Move development dependencies to separate file to improve compatibility

@parasj parasj self-assigned this Oct 25, 2022
@parasj parasj marked this pull request as ready for review October 25, 2022 19:36
@@ -41,7 +41,7 @@ Before you submit a pull request, make sure to complete the following steps:

.. code-block:: console

$ pip install black pytype autoflake
$ pip install -r requirements-dev.txt
Copy link
Contributor

Choose a reason for hiding this comment

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

what about:

poetry install --only dev

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We have an issue that Poetry requires Python version compliance with all dev dependencies and will not allow specification of a different Python version.

There's an in progress PR to add support for this, but it won't be production-ready for a while: python-poetry/poetry#5740

Several dev dependencies have not been updated for Python 3.11, most notably pytype which requires python >=3.7.1<3.11.

Until that PR lands, an alternative solution is to extract development CLI dependencies into another file. Thoughts?

pyproject.toml Outdated Show resolved Hide resolved
Copy link
Contributor

@phi-line phi-line left a comment

Choose a reason for hiding this comment

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

I'm a little confused with the use of poetry in this project. I would opt for maintaining the pyproject.toml with groups for dev and gateway instead of using a combination of poetry, pip, and requirements.txts.

@parasj
Copy link
Contributor Author

parasj commented Oct 26, 2022

Package management is a bit confusing:

  • docs/requirements.txt must exist for ReadTheDocs to install requirements in their build server
  • scripts/requirements-gateway.txt contains a stripped down set of dependencies for the Docker image build, specifically ignoring CLI/API dependencies which would bloat the Docker package size. If I can only install the gateway dependencies without the base dependencies, this could move to only use poetry
  • pip install -e . is used for development to install in editable mode. This is not supported by Poetry so a placeholder setup.py links pyproject.toml to pip. We may be able to remove setup.py with a newer poetry-core version, see [draft] Remove setup.py with editable support #640.
  • pyproject.toml is the canonical source of build dependencies. We moved to poetry to improve the release process.

@phi-line
Copy link
Contributor

Package management is a bit confusing

Thanks for the explanation!

@parasj parasj merged commit 56eebc1 into main Oct 26, 2022
@parasj parasj deleted the update-py11-gateway branch October 26, 2022 22:51
@parasj
Copy link
Contributor Author

parasj commented Oct 26, 2022

@phi-line Thanks for the details feedback on these PRs. I just added you as a contributor to the project. Are you on the Skyplane Slack? https://join.slack.com/t/skyplaneworkspace/shared_invite/zt-1cxmedcuc-GwIXLGyHTyOYELq7KoOl6Q

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.

2 participants