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

Update Python dependencies to eliminate version conflicts #6122

Merged
merged 12 commits into from
Jun 30, 2023

Conversation

justinclift
Copy link
Member

@justinclift justinclift commented Jun 26, 2023

What type of PR is this?

  • Other

Description

This PR gets all of the Python backend dependencies to work together without version mismatch errors during their pip install.

How is this tested?

  • Manually

Only tested using the basic "docker build" step, as it's a work in progress. It'll be useful to see if the CI tests are happy with it though, especially after the dql code is gone (not yet done).

@justinclift
Copy link
Member Author

justinclift commented Jun 26, 2023

Error: No such command 'list_types'.

That'll probably go away after I rip out the dql code. 😄


Nope, it wasn't dql directly causing that. Got it fixed anyway though.

@justinclift justinclift force-pushed the justin_updating_lots_of_dependencies_v1 branch 2 times, most recently from af6975f to 4842a65 Compare June 29, 2023 20:21
@justinclift
Copy link
Member Author

k, that looks like an initial minimum set of updates to get the dependencies working together without problems.

Next step is to clean it up, and see if any of the temporarily removed dependencies can be re-added. Firebolt is pretty clearly out (for now) though, due to direct incompatibility with other things.

@justinclift justinclift force-pushed the justin_updating_lots_of_dependencies_v1 branch from 4842a65 to b63fcbd Compare June 29, 2023 22:18
@justinclift
Copy link
Member Author

justinclift commented Jun 29, 2023

@wlach Would you be ok to cast an eye over this review-wise to ensure it seems sane to you?

@justinclift justinclift changed the title WIP: Updating a bunch of Python dependencies, so there are no dependency errors during docker build Update Python dependencies to eliminate version conflicts Jun 29, 2023
@justinclift justinclift requested a review from wlach June 29, 2023 22:36
@justinclift justinclift marked this pull request as ready for review June 29, 2023 22:36
@justinclift
Copy link
Member Author

justinclift commented Jun 30, 2023

Ugh. Hold off on this. I've just done a final test of the dependencies using a different process, and some further conflicts are showing up:

google-auth 2.21.0 requires urllib3<2.0, but you'll have urllib3 2.0.3 which is incompatible.
botocore 1.13.50 requires urllib3<1.26,>=1.20; python_version >= "3.4", but you'll have urllib3 2.0.3 which is incompatible.
snowflake-connector-python 3.0.4 requires urllib3<1.27,>=1.21.1, but you'll have urllib3 2.0.3 which is incompatible.
nzalchemy 11.0.2 requires SQLAlchemy<=1.3.24, but you'll have sqlalchemy 2.0.17 which is incompatible.

Dammit. k, will fix them...


It turns out those are false warnings. They're just due to the different process not actually looking at all the requirements*.txt files properly. So, this is fine to review and potentially merge after all. 😄

@justinclift justinclift marked this pull request as draft June 30, 2023 00:04
@justinclift justinclift marked this pull request as ready for review June 30, 2023 00:07
@justinclift
Copy link
Member Author

@getredash/maintainers Anyone up for reviewing this PR?

It only touches the Python backend part of things though, and doesn't have any front end changes.

Copy link
Member

@junnplus junnplus left a comment

Choose a reason for hiding this comment

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

LGTM.

@justinclift
Copy link
Member Author

Thanks heaps @junnplus. 😄

@justinclift justinclift merged commit c2c7f44 into master Jun 30, 2023
@justinclift justinclift deleted the justin_updating_lots_of_dependencies_v1 branch June 30, 2023 06:15
myonlylonely added a commit to myonlylonely/redash-custom that referenced this pull request Jul 2, 2023
* commit 'ee601ec20676b0705e352bfb9b6726a0e53e3b12':
  Fix `FEATURE_POLICY` to initialize from `REDASH_FEATURE_POLICY` envvar, not `REDASH_REFERRER_POLICY` (getredash#5822)
  Update to latest in yarn 1.22.x series (getredash#6140)
  Use precompiled psycopg2-binary package instead of psycopg2 (getredash#6137)
  Pymongo version upgrade changes (getredash#5970)
  Bump jinja2 from 2.10.3 to 2.11.3 (getredash#5431)
  Update Python dependencies to eliminate version conflicts (getredash#6122)
  Upgrade Python version to 3.8 (getredash#6130)
  Try fixing cypress in a more correct way (getredash#6117)
  ci: Attempt to fix restyled.io test (getredash#6116)
  fix pivot table style error (getredash#6112)
  dependencies: pystache==0.6.0 (getredash#6091)
  ci: migrate to github workflow ci (getredash#6008)
  Update Simba ODBC driver (getredash#6110)
@junnplus junnplus mentioned this pull request Jul 7, 2023
2 tasks
@justinclift
Copy link
Member Author

Now that we've largely updated our Python dependencies, we could probably check if that Firebolt code we removed in f7aa5b4 could be added back.

@masayuki038
Copy link
Collaborator

masayuki038 commented Nov 2, 2023

@justinclift I noticed that redash/query_runner/dynamodb_sql.py and some query runners were removed in this PR. However, the issue regarding DynamoDB was added this week 😅

Will these query_runners be unsupported from the next release? Or are there any plans to revive them?

@justinclift
Copy link
Member Author

Will these query_runners be unsupported from the next release?

From my perspective, I reckon it depends on what state their Python libaries are in.

Looking at the dynamodb library info on PyPi just now... its one and only release was over three years ago:

https://pypi.org/project/dynamodb/#history

image

The source code repository shows the same lack of continued development:

https://github.com/AllanKT/python-dynamo

That GitHub repository also shows no forks at all. 😦

So, for that one specific Python library I'd personally be ok with not including it with the next release. Which of course is going to be lousy for anyone currently using it. ☹️


There are several of other results when searching for "dynamodb" on PyPi:

https://pypi.org/search/?q=dynamodb

So, maybe it'll be possible to switch to a different Python library instead without too much effort?

It'd need someone to investigate things a bit better, and potentially do some experimenting / updating of the old DynamoDB query runner code to get it working again.

@masayuki038
Copy link
Collaborator

masayuki038 commented Nov 10, 2023

@justinclift Thank you for your reply.

Indeed, this python-dynamo has not been updated. In addition, Python version is only listed up to 3.7, so it is certainly not a good idea to keep using this library.

Is the fundamental problem dependency hell? I'm concerned that Redash is loading libraries for all datasources. I think it is very hard to keep dependencies. Introducing new datasources can also be difficult. Are there any plans to address this issue?

@justinclift
Copy link
Member Author

Are there any plans to address this issue?

Some rough ideas for fixing the problem have been talked about before, but we've not really investigated and committed to a clearly defined plan yet.

harveyrendell pushed a commit to pushpay/redash that referenced this pull request Jan 8, 2025
…6122)

* Remove DynamoDB as a data source for now, due to dependency incompatibilities
* Remove Firebolt as a data source for now, due to dependency incompatibilities
* Remove pymapd / pyomnisci / heavyai, due to dependency incompatibilities
* Remove pycrypto, upgrade cryptography
* Remove pyarrow (for now), which we no longer need
* Require an (older) version of black, to avoid test failures with more recent click library
* Upgrade gevent to 21.12.0
* Upgrade greenlet to 1.1.2
* Upgrade httpli2 to 0.18.1
* Upgrade requests to 2.31.0
* Upgrade snowflake-connector-python to 3.0.4
* Upgrade urllib3 to 1.25.11

Note, the warning message caused by the older versions of gevent and greenlet was:

    RuntimeWarning: greenlet.greenlet size changed, may indicate binary incompatibility. Expected 40 from C header, got 144 from PyObject
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.

3 participants