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

snowflake resource picker to show both snowflake + snowflake_oauth #5005

Merged
merged 1 commit into from
Jan 3, 2025

Conversation

alpetric
Copy link
Contributor

@alpetric alpetric commented Jan 2, 2025

Important

Add support for snowflake_oauth alongside snowflake in resource loading, connection testing, schema handling, and UI components.

  • Behavior:
    • ResourcePicker.svelte: Modified loadResources() to query both snowflake and snowflake_oauth resources.
    • TestConnection.svelte: Added snowflake_oauth to the scripts object for connection testing.
    • utils.ts: Updated loadTableMetaData() and scripts to handle snowflake_oauth.
  • UI Components:
    • components.ts: Added snowflake_oauth to the database type options.
    • StaticInputEditor.svelte: Updated to include snowflake_oauth in resource type checks.
  • Types:
    • inputType.ts: Added snowflake_oauth to InputType and AppInput.

This description was created by Ellipsis for ec2f895. It will automatically update as commits are pushed.

@alpetric alpetric requested a review from rubenfiszel as a code owner January 2, 2025 19:52
Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me! Reviewed everything up to ec2f895 in 1 minute and 28 seconds

More details
  • Looked at 149 lines of code in 6 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. frontend/src/lib/components/apps/components/display/dbtable/utils.ts:380
  • Draft comment:
    Ensure that checking for 'AUTOINCREMENT%' in COLUMN_DEFAULT accurately determines identity and primary key status for Snowflake. This logic might not cover all cases.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The PR introduces a new resource type 'snowflake_oauth' alongside 'snowflake'. The changes are consistent across the files, ensuring that 'snowflake_oauth' is treated similarly to 'snowflake'. However, there is a potential issue with the SQL query for 'snowflake_oauth' in the utils.ts file. The query checks for 'AUTOINCREMENT%' in COLUMN_DEFAULT to determine if a column is an identity or primary key, which might not be accurate for all cases. This logic should be verified to ensure it aligns with Snowflake's behavior.

Workflow ID: wflow_z5RGl3fBBXMi0CR7


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

Copy link

Deploying windmill with  Cloudflare Pages  Cloudflare Pages

Latest commit: ec2f895
Status: ✅  Deploy successful!
Preview URL: https://a80e3322.windmill.pages.dev
Branch Preview URL: https://alp-snowflake-resource-picke.windmill.pages.dev

View logs

@rubenfiszel rubenfiszel merged commit c999788 into main Jan 3, 2025
3 checks passed
@rubenfiszel rubenfiszel deleted the alp/snowflake_resource_picker branch January 3, 2025 15:28
@github-actions github-actions bot locked and limited conversation to collaborators Jan 3, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants