Skip to content

Patch supports_statement_cache warning.#259

Merged
Brooke-white merged 2 commits into
sqlalchemy-redshift:mainfrom
gmcrocetti:fix-supports-statement-cache-log
Dec 8, 2022
Merged

Patch supports_statement_cache warning.#259
Brooke-white merged 2 commits into
sqlalchemy-redshift:mainfrom
gmcrocetti:fix-supports-statement-cache-log

Conversation

@gmcrocetti

Copy link
Copy Markdown
Contributor

fix SQLAlchemy supports_statement_cache warning.

SQLAlchemy implementation looks into the class dictionary which is why the warning is still present even though flag is set on the Mixin base classes. The value is properly set into the base classes now.

This is why it's stated on their documentation:
image

Todos

  • MIT compatible
  • Tests
  • Documentation
  • Updated CHANGES.rst

@Brooke-white Brooke-white left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

overall looks good, let's apply this change to the redshift_connector dialect as well for consistency.

if you could add an entry to changes.rst for this PR I can go ahead and merge it

Comment thread sqlalchemy_redshift/dialect.py
…y implementation looks into the class dictionary which is why the warning is still present even though flag is set on the Mixin base classes. The value is properly set into the base classes now.
@gmcrocetti gmcrocetti force-pushed the fix-supports-statement-cache-log branch from a088238 to 7c0e80d Compare November 9, 2022 18:27
@gmcrocetti

Copy link
Copy Markdown
Contributor Author

@Brooke-white just updated the changelog.

@gmcrocetti gmcrocetti changed the title Patch for supports_statement_cache warning. Patches supports_statement_cache warning. Nov 11, 2022
@gmcrocetti gmcrocetti changed the title Patches supports_statement_cache warning. Patch supports_statement_cache warning. Nov 12, 2022
@gmcrocetti

Copy link
Copy Markdown
Contributor Author

Thanks for adding the PR link. I completely missed it :(

@Brooke-white Brooke-white merged commit d4f8fb6 into sqlalchemy-redshift:main Dec 8, 2022
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.

2 participants