Skip to content

Conversation

@nh758
Copy link
Contributor

@nh758 nh758 commented Sep 1, 2025

When using the terraform ecs/app module we get ENV variables to connect with the redis (elasticache) instance.
Here I’m mapping the ENV variables we will get to what cote is expecting.

Note this includes an update to cote, as the underlying redis library switched from node-redis to ioredis, which is configured slightly differently. Specifically the DB number.

@github-actions
Copy link
Contributor

github-actions bot commented Sep 1, 2025

Please add a release label:
major (for breaking changes), minor (for new features), patch (for bug fixes) or no_release (to skip the auto release process).

@nh758 nh758 added the minor label Sep 1, 2025
@nh758 nh758 requested a review from johnny-hausman September 1, 2025 20:27
Copy link
Collaborator

@johnny-hausman johnny-hausman left a comment

Choose a reason for hiding this comment

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

Question:

// Map Environment Variables for redis. Needed to work with the ecs/app module.
// Keep this before any requires of cote
if (
Object.keys(process.env).every((k) => !k.startsWith("COTE_DISOCOVERY_REDIS"))
Copy link
Collaborator

Choose a reason for hiding this comment

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

So am I right in interpreting this as saying, "As long as there are no env variables that start with "COTE_DISCOVERY_REDIS", then make sure these COTE env variables are set?

What if only 1 or two are set? Do we need to account for that?

I was just looking at my setup, and I only have COTE_DISCOVERY_REDIS_HOST set. If any of our SESSION_REDIS_* values are set, shouldn't those ALWAYS map to the COTE_* values?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We could do it that way. I don’t have a preference either way.
My thought was to do it this way so as not to interfere with existing deployments which will have set at least one COTE_DISCOVERY_REDIS param. But they wouldn’t have SESSION_REDIS_* set either.

Since we’re using a terraform module to set up the ecs app, will always get these 3 SESSION_REDIS params so I don’t think we need to account for that.

@nh758 nh758 requested a review from johnny-hausman September 4, 2025 01:03
@nh758
Copy link
Contributor Author

nh758 commented Sep 5, 2025

We can set these in terraform

@nh758 nh758 closed this Sep 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants