-
Notifications
You must be signed in to change notification settings - Fork 198
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
drop python 3.8, enable python 3.13, and enable full linting for 3.12 #2194
base: devel
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for dlt-hub-docs canceled.
|
368e04d
to
d0de684
Compare
d0de684
to
8cc3b7a
Compare
5e4f829
to
fc4006c
Compare
requests = ">=2.26.0" | ||
pendulum = ">=2.1.2" | ||
# TODO: pin to tag on dlt-pendulum repo | ||
dlt-pendulum = [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NOTE: this still requires cargo at this point
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rudolfix I have another question about this: should we not have a different package name for our pendulum? The way it is currently set up, dlt-pendulum is always used, also for python versions before 3.13, this means there will be package name collissions if users also have the regular pendulum in their dependencies already in a project. I'm not sure what happens in this case.
Also you mentioned that you made some changes they will not like and there is a change in the code where you removed the casting of pendulum.now() to a string, are the libraries now incompatible?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are now using a properly build dlt-pendulum for python 3.13, so that part is solved. The tests will verify wether this string removal still works or not.
7f22437
to
e46f21c
Compare
@@ -26,7 +26,7 @@ jobs: | |||
matrix: | |||
os: | |||
- ubuntu-latest | |||
python-version: ["3.8.x", "3.9.x", "3.10.x", "3.11.x"] | |||
python-version: ["3.9.x", "3.10.x", "3.11.x", "3.12.x"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
make dev works for 3.12 now!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! just resolve all the conflicts...
cca2c57
to
b3372de
Compare
@@ -1,4 +1,4 @@ | |||
FROM alpine:3.15 | |||
FROM python:3.11.11-alpine |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note: switched to a python image, I can probably revert this if we want to use a vanilla alpine here.
|
||
# install arrow 17.0.0, usually we would need apache-arrow-dev=17.0.0 but it is not available in alpine 3.20 | ||
# adapt this version to the arrow version you need | ||
RUN git clone --no-checkout https://github.com/apache/arrow.git /arrow \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as noted in the comment, alpine always seems to come with one version of each package, i remember something like this from our last company, so I need to build arrow 17 here if I want to use it..
# poetry export -f requirements.txt --output _gen_requirements.txt --without-hashes --extras gcp --extras redshift | ||
# grep `cat compiled_packages.txt` _gen_requirements.txt > compiled_requirements.txt | ||
poetry install --no-interaction -E gcp -E redshift -E duckdb | ||
poetry run pip freeze > _gen_requirements.txt |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this works well, poetry export was failing on several dependencies..
Description
This PR does the following:
Based on #2047
ToDo: