Snowflake-Labs / django-snowflake

MIT License
59 stars 15 forks source link

ROLE should be a top level config #68

Closed sfc-gh-hachouraria closed 1 year ago

sfc-gh-hachouraria commented 1 year ago

In practice, the concept of a USER (i.e. their DEFAULT_ROLE) is ambiguous in Snowflake.

Currently role can be passed through OPTIONS but this approach does not surface the control very well.

It should be a top level config to be explicit on what role will be used to perform all database activity.

For convenience the config can remain optional (to rely on DEFAULT_ROLE of the USER when unset).

timgraham commented 1 year ago

I initially implemented ROLE as a top level option before d3833b0670a7518a5bd79eb2113fbd6350c8ee27. As I remember, I moved it because it's an optional value and I didn't like that there would be two possible places to specify it (top level and in OPTIONS). I guess the same argument about "two possible places to specify" could be made for other Snowflake-specific top-level options like ACCOUNT, SCHEMA, and WAREHOUSE, even as these are required options. I suppose I could be convinced either way on this.

sfc-gh-hachouraria commented 1 year ago

Thank you for taking a look at this request Tim. We could keep it as-is and simply document it as another popular option in https://github.com/Snowflake-Labs/django-snowflake/pull/66?

timgraham commented 1 year ago

That sounds helpful. Thanks.