Skip to content

Conversation

@vdonato
Copy link
Collaborator

@vdonato vdonato commented Apr 19, 2023

There's a bunch of tiny fixes that came out of testing st.experimental_connection that
are small enough that it's not worth the overhead of making individual PRs for.

This PR batches these changes. Note it's easiest to review this commit-by-commit.

@vdonato vdonato added the security-assessment-completed Security assessment has been completed for PR label Apr 19, 2023
@vdonato vdonato marked this pull request as ready for review April 19, 2023 23:30
@vdonato vdonato requested a review from lukasmasuch April 19, 2023 23:30
@vdonato vdonato force-pushed the vdonato/misc-connection-fixes branch from c35b223 to ee35111 Compare April 19, 2023 23:31
@vdonato vdonato force-pushed the vdonato/misc-connection-fixes branch from ee35111 to d8aaa3e Compare April 19, 2023 23:36
Copy link
Collaborator

@lukasmasuch lukasmasuch left a comment

Choose a reason for hiding this comment

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

LGTM 👍

)
@cache_data(ttl=ttl)
@cache_data(
show_spinner="Running `SQL.query(...)`.",
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: maybe sql.query(...) would be slightly better here since it's more likely to be something called on an instance, and the lowercase version also corresponds better to the connection name.

@vdonato vdonato merged commit 23900ac into feature/st.experimental_connection Apr 20, 2023
@vdonato vdonato deleted the vdonato/misc-connection-fixes branch April 20, 2023 05:42
vdonato added a commit that referenced this pull request Apr 20, 2023
* Add missing default arg

* Error when tenacity is not installed on connection creation

* Add more missing module -> PyPI package mappings

* Change spinner messages for .query() methods to hide internals

* Complain more clearly if no SQL connection params at all are set

* Support named snowsql config file sections

* Remove unneeded f-string

* Raise exception when no Snowpark config is set

* Use correct mysql PyPI package name

* Make query method spinner messages lowercase
vdonato added a commit that referenced this pull request Apr 21, 2023
* Add missing default arg

* Error when tenacity is not installed on connection creation

* Add more missing module -> PyPI package mappings

* Change spinner messages for .query() methods to hide internals

* Complain more clearly if no SQL connection params at all are set

* Support named snowsql config file sections

* Remove unneeded f-string

* Raise exception when no Snowpark config is set

* Use correct mysql PyPI package name

* Make query method spinner messages lowercase
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

security-assessment-completed Security assessment has been completed for PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants