Skip to content
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

[BugFix]: Creating a PostgresBaseEngineSpec so changes to the Postgre… #4224

Merged
merged 2 commits into from Feb 4, 2018

Conversation

fabianmenges
Copy link
Contributor

…sEngineSpec don't affect every subclass

Refactored all classes that inherit from PostgresEngineSpec to inherit from PostgresBasesEngineSpec instead and moved them all to one place in the file.

#3856 Broke get_table_names for Vertica and possibly Redshift and Oracle

Copy link
Contributor Author

@fabianmenges fabianmenges left a comment

Choose a reason for hiding this comment

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

Please do not merge! There seem to be more issues (e.g. refreshing the metadata)

@mistercrunch
Copy link
Member

@fabianmenges let me know when this is ready to merge

@fabianmenges
Copy link
Contributor Author

This is ready to merge and also addresses a small part of #4230
This was masking some other issues with the Vertica connector that I fixed as part of this PR LocusEnergy/sqlalchemy-vertica-python#11

class PostgresBaseEngineSpec(BaseEngineSpec):
""" Abstract class for Postgres 'like' databases """

engine = 'postgresqlbase'
Copy link
Contributor

@xrmx xrmx Jan 18, 2018

Choose a reason for hiding this comment

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

Shouldn't this be empty (no engine attribute) so people extending it have to remember to add something sensible?

@fabianmenges
Copy link
Contributor Author

Ping, this is still required for Vertica support.

@mistercrunch mistercrunch merged commit a9e1e68 into apache:master Feb 4, 2018
michellethomas pushed a commit to michellethomas/panoramix that referenced this pull request May 24, 2018
apache#4224)

* [BugFix]: Creating a PostgresBaseEngineSpec so changes to the PostgresEngineSpec don't affect every subclass

* Empty engine for abstract Engine
wenchma pushed a commit to wenchma/incubator-superset that referenced this pull request Nov 16, 2018
apache#4224)

* [BugFix]: Creating a PostgresBaseEngineSpec so changes to the PostgresEngineSpec don't affect every subclass

* Empty engine for abstract Engine
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.23.0 labels Feb 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.23.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants