-
Notifications
You must be signed in to change notification settings - Fork 22
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
[Issue 2665] Add search path to db connection params #2836
Conversation
@@ -52,6 +52,7 @@ def get_connection_parameters(config: DBSettings) -> dict[str, Any]: | |||
token = ( | |||
config.password if config.local_env is True else generate_iam_auth_token(config) | |||
) | |||
search_path = "public" if config.local_env is True else "app" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd put this more explicitly in an environment variable in the config class itself.
Something like:
db_schema: str = Field("app", alias="DB_SCHEMA")
Then in local.env, put DB_SCHEMA=public
This makes it much easier to adjust or configure.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done! Please see latest commit
@@ -17,6 +17,7 @@ class DBSettings(PydanticBaseEnvConfig): | |||
user: str = Field (alias="DB_USER") | |||
password: Optional[str] = Field(None, alias="DB_PASSWORD") | |||
ssl_mode: str = Field("require", alias="DB_SSL_MODE") | |||
db_schema: str = Field ("app", alias="DB_SCHEMA") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You would need to set something in local.env as well to override this to be public
locally.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did that locally but did not commit the changes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll push it up now
Summary
Fixes #2665
Time to review: 1 min
Changes proposed
Added search path to db connection params.
Context for reviewers
This is latest step in a series of attempts to resolve failed db connections and privileges from /analytics step functions. See ticket history and comments for details.
Additional information