-
Notifications
You must be signed in to change notification settings - Fork 1.6k
[MINORBUMP] Bump SQLA minimum version to 2.0 #11413
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
base: develop
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for niobium-lead-7998 canceled.
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #11413 +/- ##
===========================================
- Coverage 84.09% 84.07% -0.02%
===========================================
Files 468 468
Lines 39005 39005
===========================================
- Hits 32801 32795 -6
- Misses 6204 6210 +6
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
e78e3d4 to
f6afbf8
Compare
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.
Pull Request Overview
This PR bumps the minimum SQLAlchemy version from 1.4.0 to 2.0, removing all SQLAlchemy 1.x compatibility code and dependencies. This is a significant maintenance change that modernizes the codebase to use only SQLAlchemy 2.x.
Key changes include:
- Removed all SQLAlchemy 1.x compatibility files and references
- Updated minimum version constraints to require SQLAlchemy 2.0+
- Consolidated SQLAlchemy 2.x requirements into the main requirements files
- Updated test configurations and CI pipelines to reflect the new minimum version
Reviewed Changes
Copilot reviewed 24 out of 24 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/test_packaging.py | Removed references to sqlalchemy1 requirements and updated package count assertions |
| tasks.py | Removed sqlalchemy1 requirement files from test dependencies and updated redshift references |
| setup.py | Removed SQLAlchemy 1.x compatibility logic and consolidated to SQLAlchemy 2.x requirements |
| requirements-types.txt | Updated to use sqlalchemy.txt instead of sqlalchemy2.txt and added version constraints |
| reqs/ files | Removed SQLAlchemy 1.x files, updated version constraints, and consolidated requirements |
| pyproject.toml | Removed SQLAlchemy type checking exclusion |
| compatibility files | Cleaned up type ignore comments related to SQLAlchemy compatibility |
| docs/ | Updated documentation to reference new package names |
| ci/ files | Updated minimum version constraints and CI pipeline configurations |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| numpy==1.23.0 | ||
| pandas==1.5.1 | ||
| sqlalchemy<2.0.0 # SQLAlchemy 2.0.0 is not compatible with pandas < 2.0.0 | ||
| numpy==1.23.2 |
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.
Pandas 2.0.0 has different numpy requirements. Python 3.11 requires numpy>=1.23.2
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.
Pull Request Overview
Copilot reviewed 24 out of 24 changed files in this pull request and generated 1 comment.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
...ersion-0.18/oss/guides/connecting_to_your_data/database/components/_redshift_dependencies.md
Show resolved
Hide resolved
c41cb1a to
99f451d
Compare
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.
Instead of deleting I would change this to look like requirements-dev-gx-redshift.txt. I think that would like people do pip install great_expectations[redshift] and get our redshift. We should verify that we can install our redshift package in that way.
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 already have a requirements-dev-gx-redshift.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.
I believe that requires someone doing pip install great_expectations[gx-redshift]. Both should work. People will most likely do pip install great_expectations[redshift] since that is more like what one would do for other databases but we don't want to break anyone who does [gx-redshift].
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.
Got it, let me check.
This PR drops support for
SQLAlchemy < 2and replaces the requirement forsqlalchemy-redshiftwithgx-sqlalchemy-redshift.invoke lint(usesruff format+ruff check)For more information about contributing, visit our community resources.
After you submit your PR, keep the page open and monitor the statuses of the various checks made by our continuous integration process at the bottom of the page. Please fix any issues that come up and reach out on Slack if you need help. Thanks for contributing!