-
Notifications
You must be signed in to change notification settings - Fork 3
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
fix: upgrade python requirements #596
Conversation
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.
Great, thanks for getting to the bottom of this.
On the SQLAlchemy thing, I think the right thing to do here is to add
supports_statement_cache = True
to all our dialect subclasses. From looking at the the docs you linked to it seems to be a question of whether the dialect hardcodes literal parameters into SQL strings during compilation, and we're not doing anything like that. But in any case, you're right to address this in a separate ticket.
Another thing we should look at is testing our databricks compatibility. Unfortunately we can't do this in CI, but Simon has automated this as far as possible and written some detailed instructions.
I notice we're jumping from v0.9.1 to v2.0.2 of databricks-sql-connector
so now would be a good time to do this. But I don't think testing should block this PR though. We may well have already broken compatibility with all the refactoring we've been doing, so we should move to the latest version and then figure out how to make that work — if it doesn't already.
Ah - I did notice this, but I didn't realise that bit wasn't tested fully. I'll have a look at it separately to this PR. I think the docs are out of date too, the test referred to there doesn't exist |
c6880ab
to
9600d95
Compare
Sorry @evansd - I'd missed the requirements.dev.txt from the previous commit, and based on discussion in this thread I think the empty requirements.prod.in might be the thing stopping dependabot from working properly for prod requirements, so I've deleted it |
Ah, interesting |
But actually, maybe not. In any case, it probably makes sense to remove the empty |
Fixes #579
Upgrade all prod and dev requirements except sqlalchemy**
This addresses the current security alerts for numpy and pyjwt.
Note that it appears dependabot doesn't currently support pyproject.toml outside of poetry - see this issue, so prod dependencies are not being updated as expected.
** sqlachemy is now pinned to 1.4.27; upgrading past 1.4.27 results in warnings which cause test errors, e.g.:
All our engine dialects inherit from base classes that include
supports_statement_cache = True
, but as of v1.4.28 (not 1.4.5 as it says in the docs), sqlalchemy requires third party dialects to explicitly declare that too.As I'm not entirely sure what we need to do to check and test that our subclassed dialects fully support caching, I've just pinned sqlalchemy for now, and I'll make another ticket for the caching support.