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

[Issue 2665] Update analytics db settings to recognize empty pwd setting in env #2796

Merged
merged 8 commits into from
Nov 8, 2024

Conversation

DavidDudas-Intuitial
Copy link
Collaborator

@DavidDudas-Intuitial DavidDudas-Intuitial commented Nov 8, 2024

Summary

Fixes #2665

Time to review: 1 min

Changes proposed

What was added, updated, or removed in this PR.

Made DBSettings.password an optional field in analytics/config.py, thereby allowing the value to be None, which in turn allows IAM token-creation to occur.

Context for reviewers

Testing instructions, background context, more in-depth details of the implementation, and anything else you'd like to call out or ask reviewers. Explain how the changes were verified.

Additional information

Screenshots, GIF demos, code examples or output to help show the changes working as expected.

Copy link
Collaborator

@coilysiren coilysiren left a comment

Choose a reason for hiding this comment

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

I assume the idea is: you do use the password locally, and don't in AWS, therefore the password must be optional

@DavidDudas-Intuitial
Copy link
Collaborator Author

DavidDudas-Intuitial commented Nov 8, 2024

I assume the idea is: you do use the password locally, and don't in AWS, therefore the password must be optional

That's exactly right. When running in AWS, the ENV should presumably have a pwd value of None. The code I merged yesterday (#2786) has logic to generate an IAM token if the pwd value is None.

@DavidDudas-Intuitial DavidDudas-Intuitial merged commit ce2902c into main Nov 8, 2024
6 checks passed
@DavidDudas-Intuitial DavidDudas-Intuitial deleted the issue-2665-implement-iam branch November 8, 2024 20:09
DavidDudas-Intuitial added a commit that referenced this pull request Nov 9, 2024
…wd (#2799)

## Summary
Fixes #2665 

### Time to review: __2 mins__

## Changes proposed
> What was added, updated, or removed in this PR.

Adds a switch in `analytics/integrations/db.py` that determines which
value to use for a DB password when connecting to Postgres: either the
value in `local.env` or an IAM token, depending on an environment
variable.

Also adds better exception handling, to make errors easier to spot in
CI.

## Context for reviewers
> Testing instructions, background context, more in-depth details of the
implementation, and anything else you'd like to call out or ask
reviewers. Explain how the changes were verified.

This is a follow up to previous PRs,
#2786 and
#2796, and part of an
effort to get `analytics` step functions to successfully connect to
Postgres DB in a CI environment.

## Additional information
> Screenshots, GIF demos, code examples or output to help show the
changes working as expected.
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.

Load GitHub data into Postgres in Dev
2 participants