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(mysql): get_model now supports extra kwargs so it doesnt crash [TCTC-9223] #1764

Merged
merged 4 commits into from
Sep 10, 2024

Conversation

fspot
Copy link
Member

@fspot fspot commented Sep 10, 2024

Laputa now calls get_model with some extra parameters, but these parameters were not added for 3 sql connector's get_model method :

  • mysql
  • redshift
  • snowflake oauth2

When get_model fails, the screen supposed to list db tree structure loads forever (cf. jira card https://toucantoco.atlassian.net/browse/TCTC-9223)

@fspot fspot added the bug Something isn't working label Sep 10, 2024
@fspot fspot self-assigned this Sep 10, 2024
@fspot fspot changed the title fix(mysql): get_model now supportsextra kwargs so it doesnt crash [TCTC-9223] fix(mysql): get_model now supports extra kwargs so it doesnt crash [TCTC-9223] Sep 10, 2024
Copy link
Member

@PrettyWood PrettyWood left a comment

Choose a reason for hiding this comment

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

Putain je suis con je pensais avoir remonté ce point à Luka j'y avais pensé pendant la review 🤦 Pardon

Copy link
Contributor

@lukapeschke lukapeschke left a comment

Choose a reason for hiding this comment

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

My bad I was 100 sure that I did the update everywhere 🤦

Could we maybe enable type checking on these files if it isn't too much work ?

@fspot fspot merged commit 56718da into master Sep 10, 2024
4 checks passed
@fspot fspot deleted the fix-get-model-mysql branch September 10, 2024 14:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants