build(ingest): upgrade to sqlalchemy 1.4, drop 1.3 support #8810
Conversation
github-actions
bot
added
ingestion
| @@ -451,17 +451,10 @@ def _get_operation_aspect_work_unit( | |||
| yield wu | |||
|
|
|||
| def _process_snowflake_history_row( | |||
| self, row: Any | |||
| self, event_dict: dict | |||
There was a problem hiding this comment.
We use DictCursor in snowflake, so this is always a dictionary.
metadata-ingestion/setup.py
Outdated
| @@ -112,7 +112,7 @@ def get_long_description(): | |||
|
|
|||
| sql_common = { | |||
| # Required for all SQL sources. | |||
| "sqlalchemy>=1.3.24, <2", | |||
| "sqlalchemy>=1.4.0, <2", | |||
There was a problem hiding this comment.
Do we actually support 1.4.0? We probably want a stronger lower bound
| "sqlalchemy>=1.4.0, <2", | |
| "sqlalchemy>=1.4.0, <2", |
There was a problem hiding this comment.
I noticed lint failure in ./tests/test_helpers/sqlalchemy_mypy_plugin.py when using sqlalchemy==1.4.0. Upgrading to 1.4.8 lower bound solves this. Do you have any thoughts on how to decide stronger lower bound - maybe based on recency OR quickly try out some sqlalchemy versions in test matrix and choosing the least which works .
There was a problem hiding this comment.
I suspect our true lower bound is higher than 1.4.8, but I don't think it's worth the effort of finding it just yet
For now, let's set the lower bound to something that's a year or so old (maybe 1.4.39 from Jun 2022) and add a comment explaining that it's a temporary lower bound that we're open to loosening/tightening as requirements show up
main compat issues I anticipate are gonna be from GE, so let's see how it goes
There was a problem hiding this comment.
updated lower bound to 1.4.39 and added comment in this commit.
| @@ -451,17 +451,10 @@ def _get_operation_aspect_work_unit( | |||
| yield wu | |||
|
|
|||
| def _process_snowflake_history_row( | |||
| self, row: Any | |||
| self, event_dict: dict | |||
hsheth2
added
merge-pending-ci
Updated SQLAlchemy lower bound to 1.4.8 to avoid the error `cannot import name 'Optional' from 'mypy.plugin'` in `./tests/test_helpers/sqlalchemy_mypy_plugin.py`
hsheth2
added
the
merge-pending-ci
Checklist