Skip to content

refactor: remove database_type variable and update connection string …#31

Merged
alismx merged 1 commit intomainfrom
alis/to17/db_type
May 27, 2025
Merged

refactor: remove database_type variable and update connection string …#31
alismx merged 1 commit intomainfrom
alis/to17/db_type

Conversation

@alismx
Copy link
Copy Markdown
Collaborator

@alismx alismx commented May 27, 2025

Changes Proposed

  • Updated conditional logic in _local.tf to set database variables on values passed in, not the database_type variable.

Testing

  • Verify that the README documentation is updated and the database_type input is removed.
  • Confirm that environment variables for database connection strings and credentials are set correctly based on the secrets_manager version inputs.

…conditionals

- Remove the unused database_type variable definition.
- Update local.tf to use secrets_manager versions for connection strings instead of database_type checks.
- Adjust README.md to remove documentation for the database_type input.
@alismx alismx marked this pull request as ready for review May 27, 2025 17:51
@alismx alismx merged commit b151fbd into main May 27, 2025
2 checks passed
Comment thread _local.tf
registry_password = data.aws_ecr_authorization_token.this.password
dibbs_repo = var.dibbs_repo
database_url = var.database_type == "postgresql" ? {
database_url = var.secrets_manager_postgresql_connection_string_version != "" ? {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

should we rename the variable to secrets_manager_database_connection_string_version or similar? since it isn't used only for postgres anymore

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Great suggestion, I made this change in this follow-up! #32

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.

2 participants