-
Notifications
You must be signed in to change notification settings - Fork 0
Update SQLAlchemy and Add uv support #1
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: main
Are you sure you want to change the base?
Conversation
- Add pyproject.toml with uv configuration and project metadata - Update CI buildspec to use uv instead of tox - Update README with uv installation and usage instructions - Maintain backward compatibility with existing tox workflow
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 modernizes the sqlalchemy-redshift package by updating it for SQLAlchemy 2.x compatibility and adding uv package manager support. The changes include migrating from deprecated SQLAlchemy 1.x APIs to their 2.x equivalents, replacing setup.py with pyproject.toml configuration, and updating the development toolchain.
Key changes:
- Updates SQLAlchemy API calls from 1.x to 2.x syntax (select statements, column access, imports)
- Replaces tox with uv for dependency management and testing
- Migrates from setup.py to pyproject.toml with modern Python packaging standards
Reviewed Changes
Copilot reviewed 35 out of 36 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| pyproject.toml | Modern packaging configuration replacing setup.py |
| sqlalchemy_redshift/dialect.py | Core dialect updates for SQLAlchemy 2.x compatibility |
| tests/*.py | Test updates for new SQLAlchemy 2.x syntax |
| ci/*.yml | CI pipeline updates for uv and Redshift Serverless |
| .github/workflows/*.yml | New GitHub Actions workflows for security, releases |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| In SQLAlchemy 2.0, the parent _get_column_info method was removed, | ||
| so we implement the logic directly. |
Copilot
AI
Sep 5, 2025
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.
The docstring has incomplete information about the method parameters. It should document all parameters including domains, enums, and **kwargs for API clarity.
| In SQLAlchemy 2.0, the parent _get_column_info method was removed, | |
| so we implement the logic directly. | |
| In SQLAlchemy 2.0, the parent _get_column_info method was removed, | |
| so we implement the logic directly. | |
| Parameters | |
| ---------- | |
| name : str | |
| The name of the column. | |
| format_type : str | |
| The PostgreSQL format type string for the column (e.g., 'varchar(30)', 'integer'). | |
| default : str or None | |
| The default value for the column, if any. | |
| notnull : bool | |
| Whether the column is NOT NULL. | |
| domains : dict | |
| Domain information for the column, if any. | |
| enums : dict | |
| Enum information for the column, if any. | |
| schema : str, optional | |
| The schema name, if applicable. | |
| encode : str, optional | |
| Redshift-specific encoding for the column, if any. | |
| comment : str, optional | |
| Comment for the column, if any. | |
| identity : dict, optional | |
| Identity information for the column (SQLAlchemy 2.0+). | |
| **kwargs | |
| Additional keyword arguments. | |
| Returns | |
| ------- | |
| dict | |
| A dictionary containing column information suitable for SQLAlchemy Table reflection. |
| coltype = VARCHAR # Use VARCHAR as default | ||
| except (KeyError, AttributeError): | ||
| coltype = VARCHAR # fallback for unknown types |
Copilot
AI
Sep 5, 2025
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.
Using VARCHAR as a fallback for unknown types may not be appropriate for all Redshift data types. Consider using a more generic type like NullType or raising an informative error for unsupported types.
| coltype = VARCHAR # Use VARCHAR as default | |
| except (KeyError, AttributeError): | |
| coltype = VARCHAR # fallback for unknown types | |
| coltype = NullType # Use NullType as default for unknown types | |
| except (KeyError, AttributeError): | |
| coltype = NullType # fallback for unknown types |
| AS SELECT t1.id, t1.name FROM t1 | ||
| """ | ||
| for key in ("id", selectable.c.id): | ||
| for key in ("id", selectable.selected_columns.id): |
Copilot
AI
Sep 5, 2025
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.
The use of selected_columns appears to be an incorrect API for SQLAlchemy 2.x. This should likely be selectable.c.id or selectable.columns.id to properly access column objects in SQLAlchemy 2.x.
| AS SELECT t1.id, t1.name FROM t1 | ||
| """ | ||
| for key in ("id", selectable.c.id): | ||
| for key in ("id", selectable.selected_columns.id): |
Copilot
AI
Sep 5, 2025
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.
The use of selected_columns appears to be an incorrect API for SQLAlchemy 2.x. This should likely be selectable.c.id or selectable.columns.id to properly access column objects in SQLAlchemy 2.x.
| AS SELECT t1.id, t1.name FROM t1 | ||
| """ | ||
| for key in ("id", selectable.c.id): | ||
| for key in ("id", selectable.selected_columns.id): |
Copilot
AI
Sep 5, 2025
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.
The use of selected_columns appears to be an incorrect API for SQLAlchemy 2.x. This should likely be selectable.c.id or selectable.columns.id to properly access column objects in SQLAlchemy 2.x.
| access_key_id=access_key_id, | ||
| secret_access_key=secret_access_key, | ||
| format='JSON', | ||
| format=Format.json, |
Copilot
AI
Sep 5, 2025
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.
The use of Format.json, Compression.lzop, etc. suggests these are enum values, but there's no import or definition visible for these constants in the diff. Ensure these enum classes are properly imported and defined.
| format=Format.json, | |
| format=dialect.Format.json, |
| access_key_id=access_key_id, | ||
| secret_access_key=secret_access_key, | ||
| compression='LZOP', | ||
| compression=Compression.lzop, |
Copilot
AI
Sep 5, 2025
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.
The use of Format.json, Compression.lzop, etc. suggests these are enum values, but there's no import or definition visible for these constants in the diff. Ensure these enum classes are properly imported and defined.
| access_key_id=access_key_id, | ||
| secret_access_key=secret_access_key, | ||
| compression='BZIP2', | ||
| compression=Compression.bzip2, |
Copilot
AI
Sep 5, 2025
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.
The use of Format.json, Compression.lzop, etc. suggests these are enum values, but there's no import or definition visible for these constants in the diff. Ensure these enum classes are properly imported and defined.
| secret_access_key=secret_access_key, | ||
| manifest=True, | ||
| format='JSON', | ||
| format=Format.json, |
Copilot
AI
Sep 5, 2025
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.
The use of Format.json, Compression.lzop, etc. suggests these are enum values, but there's no import or definition visible for these constants in the diff. Ensure these enum classes are properly imported and defined.
| format=Format.json, | ||
| path_file='s3://mybucket/data/jsonpath.json', | ||
| compression='GZIP', | ||
| compression=Compression.gzip, |
Copilot
AI
Sep 5, 2025
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.
The use of Format.json, Compression.lzop, etc. suggests these are enum values, but there's no import or definition visible for these constants in the diff. Ensure these enum classes are properly imported and defined.
Adds SQLAlechemy 2.x
Add uv support