Merge config parameters from various sources in SQL/Snowpark connections #6486
Conversation
|
Hey, @vdonato ! A potential improvement could be the usage of It is one of the main usages for ChainMaps to merge config options, so they fit here perfectly. Feel free to refactor this with ChainMaps usage or keep it as is. |
| database=conn_params.get("database"), | ||
| ) | ||
|
|
||
| eng = sqlalchemy.create_engine(url, **kwargs) |
There was a problem hiding this comment.
snowflake-sqlalchemy uses a connect_args kwarg outside the URL to specify common behaviors like non-password based authentication (rough cite - this is also what I found when I tried it out).
Similarly, bigquery-sqlalchemy uses a credentials_info kwarg which seems to be required for any usage (cite).
It seems like there's no way to get that passed into create_engine directly from secrets with this implementation. So to use these dialects (which are pretty important!) I still have to do something like:
conn = st.connection('sql', connect_args=dict(st.secrets.snowflakeargs))
I would love to be able to just specify a secrets section like [connections.sql.connect_args] and have it get passed through.
There was a problem hiding this comment.
(Should I move this comment to the main feature PR? Since I guess it doesn't need to be addressed in this PR)
There was a problem hiding this comment.
Ack -- will see if I can address this early next week
|
Good by me to merge and address the other comment in a separate PR if desired. |
…ons (#6486) * Add some utility functions for manipulating dicts * Teach the SQL connection to merge params set via secrets.toml and kwargs * Merge secrets from various sources in Snowpark connection * Add a @pytest.mark.require_snowflake decorator * Remove "password" from required params * Use ChainMap instead of custom helper function
…ons (#6486) * Add some utility functions for manipulating dicts * Teach the SQL connection to merge params set via secrets.toml and kwargs * Merge secrets from various sources in Snowpark connection * Add a @pytest.mark.require_snowflake decorator * Remove "password" from required params * Use ChainMap instead of custom helper function
…ons (#6486) * Add some utility functions for manipulating dicts * Teach the SQL connection to merge params set via secrets.toml and kwargs * Merge secrets from various sources in Snowpark connection * Add a @pytest.mark.require_snowflake decorator * Remove "password" from required params * Use ChainMap instead of custom helper function
…ons (#6486) * Add some utility functions for manipulating dicts * Teach the SQL connection to merge params set via secrets.toml and kwargs * Merge secrets from various sources in Snowpark connection * Add a @pytest.mark.require_snowflake decorator * Remove "password" from required params * Use ChainMap instead of custom helper function
…ons (#6486) * Add some utility functions for manipulating dicts * Teach the SQL connection to merge params set via secrets.toml and kwargs * Merge secrets from various sources in Snowpark connection * Add a @pytest.mark.require_snowflake decorator * Remove "password" from required params * Use ChainMap instead of custom helper function
📚 Context
Another small-ish thing on our TODO list was to merge the connection config params we receive
from various sources. This PR does this for both our SQL and Snowpark first party connections, and
we additionally write a few small helper functions to help with this that are likely to be useful for many
other connections.