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

fix(superset): get_database #211

Merged
merged 1 commit into from
May 31, 2023
Merged

fix(superset): get_database #211

merged 1 commit into from
May 31, 2023

Conversation

betodealmeida
Copy link
Member

Due to a CVE, some of the information available in api/v1/database/$id was moved to api/v1/database/$id/connection. This PR updates the Superset client to try both endpoints.

Added a test covering the case.

response = self.session.get(url)
validate_response(response)

resource = response.json()["result"]

Choose a reason for hiding this comment

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

it will be nice to have a schema for the return object

Copy link

@zephyring zephyring May 30, 2023

Choose a reason for hiding this comment

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

I am not familiar with the above 2 endpoints but another option is to introduce a new method for the connection endpoint.
The API client should be as dumb as possible otherwise changes from the API could easily break this client again.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point, I have a TODO in that file to add them. It's not super trivial because the schema depends on the version of Superset running.

@betodealmeida betodealmeida merged commit 8a0060c into main May 31, 2023
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