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

Override check schema exists #452

Closed

Conversation

jiezhen-chen
Copy link
Contributor

@jiezhen-chen jiezhen-chen commented May 17, 2023

resolves #555

Migrating from postgres__list_schema macro and postgres__check_schema_exists macro to directly calling redshift python connector, which has built in metadata calls that retrieve the same information. Redshift_connector call used in this PR is get_schemas.

Description

Checklist

@cla-bot cla-bot bot added the cla:yes label May 17, 2023
@jiezhen-chen jiezhen-chen marked this pull request as ready for review May 23, 2023 18:16
@jiezhen-chen jiezhen-chen requested a review from a team as a code owner May 23, 2023 18:16
@jiezhen-chen jiezhen-chen requested a review from Fleid May 23, 2023 18:16
@mikealfare mikealfare removed the request for review from Fleid August 3, 2023 16:01
@@ -142,9 +142,6 @@ def _connection_keys(self):
"schema",
"sslmode",
"region",
"sslmode",
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason to remove these? Is it related to this specific change?

def _get_cursor(self):
return self.connections.get_thread_connection().handle.cursor()

def list_schemas(self, database: str, schema: Optional[str] = None) -> List[str]:
Copy link
Contributor

Choose a reason for hiding this comment

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

I would update the associated macro to call this, otherwise any other macro that is using the associated macro would still use the postgres macro, hence pg_ tables.

return results

@available
def check_schema_exists(self, database: str, schema: str) -> bool:
Copy link
Contributor

Choose a reason for hiding this comment

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

Same comment as on list_schemas

@@ -0,0 +1,5 @@
kind: Features
Copy link
Contributor

Choose a reason for hiding this comment

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

This is missing a description that identifies the change.

Copy link
Contributor

This PR has been marked as Stale because it has been open with no activity as of late. If you would like the PR to remain open, please comment on the PR or else it will be closed in 7 days.

Copy link
Contributor

This PR has been marked as Stale because it has been open with no activity as of late. If you would like the PR to remain open, please comment on the PR or else it will be closed in 7 days.

@github-actions github-actions bot added the Stale label Aug 28, 2024
Copy link
Contributor

github-actions bot commented Sep 4, 2024

Although we are closing this PR as stale, it can still be reopened to continue development. Just add a comment to notify the maintainers.

@github-actions github-actions bot closed this Sep 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[ADAP-726] [Feature] Remove references to pg catalogs in hard-coded queries
4 participants