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

Remove imp dependency and test on python 3.12 #673

Merged
merged 5 commits into from
Oct 5, 2023
Merged

Remove imp dependency and test on python 3.12 #673

merged 5 commits into from
Oct 5, 2023

Conversation

mgyucht
Copy link
Contributor

@mgyucht mgyucht commented Oct 4, 2023

To enable users to use the databricks-cli library with Python 3.12, we must remove our use of the imp library. This is done in this PR as well as adding 3.12 to unit test coverage to ensure the package can be installed & used with Python 3.12.

@codecov-commenter
Copy link

codecov-commenter commented Oct 4, 2023

Codecov Report

Attention: 4 lines in your changes are missing coverage. Please review.

Files Coverage Δ
setup.py 0.00% <0.00%> (ø)

📢 Thoughts on this report? Let us know!.

@@ -62,6 +62,7 @@ jobs:
matrix:
python-version:
- '3.7'
- '3.12'
Copy link
Contributor

Choose a reason for hiding this comment

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

We could also run this on all versions we support to be sure.

@pietern
Copy link
Contributor

pietern commented Oct 4, 2023

@mgyucht We might need to update the runner image to ubuntu-22.04 to make it work.

Copy link

@jmahlik jmahlik left a comment

Choose a reason for hiding this comment

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

Currently there are only wheels for python 2 on pypi.

https://pypi.org/project/databricks-cli/#files

Can the universal option be added to the setup.py? This will produce a python 2 and 3 compatible wheel.

setup(
...,
options={'bdist_wheel': {'universal': True}},
)

Current -- users get the sdist, which causes setup.py invocation, leading to this issue.


databricks_cli-0.17.8-py2-none-any.whl
databricks-cli-0.17.9.dev0.tar.gz

With universal - python 2 and 3 users get a wheel which is faster to install and doesn't invoke the setup.py.

databricks_cli-0.17.9.dev0-py2.py3-none-any.whl
databricks-cli-0.17.9.dev0.tar.gz

Also related to #631.

@mgyucht
Copy link
Contributor Author

mgyucht commented Oct 4, 2023

@jmahlik Yes, we've discussed and will do that in a follow-up PR.

@mgyucht mgyucht merged commit 7f5cafd into main Oct 5, 2023
13 checks passed
@mgyucht mgyucht deleted the remove-imp branch October 5, 2023 08:18
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.

4 participants