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

Add more parameters for connection add #218

Merged
merged 2 commits into from
Jun 30, 2023

Conversation

sfc-gh-turbaszek
Copy link
Contributor

Pre-review checklist

  • I've confirmed that instructions included in README.md are still correct after my changes in the codebase.
  • I've added new automated tests to verify correctness of my new code.
  • I've confirmed that my changes are up-to-date with the target branch.
  • I've described my changes in the section below.

Changes description

Add more parameters to snow connection add to support all basic options for connection configuration.

}
print(connection_entry)
Copy link
Contributor

Choose a reason for hiding this comment

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

debug print? 😄

prompt="Snowflake account",
help="Name assigned to your Snowflake account.",
help="Account name to be used to authenticate with Snowflake.",
show_default=False,
Copy link
Contributor

Choose a reason for hiding this comment

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

What happened with account? Why removed prompt?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch, fixed

click_type=OptionalPrompt(),
prompt="Snowflake region",
help="Region name if not the default Snowflake Database deployment.",
),
Copy link
Contributor

Choose a reason for hiding this comment

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

First time see region, do we need this one?

What about role, warehouse and protocol?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yup, region is supported by snowflake python connector. Good point about role and wh which are more important. Added!

Copy link
Contributor

Choose a reason for hiding this comment

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

My eyes bleeding looking at parameters and connection_entry order but iiyo

@sfc-gh-turbaszek sfc-gh-turbaszek merged commit 32ff019 into main Jun 30, 2023
@sfc-gh-turbaszek sfc-gh-turbaszek deleted the extend-connection-options branch June 30, 2023 14:07
sfc-gh-sichen pushed a commit that referenced this pull request Oct 17, 2024
* Add more parameters for connection add

* fixup! Add more parameters for connection add
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