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

adding redis to local development #3580

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

adding redis to local development #3580

wants to merge 1 commit into from

Conversation

heckj
Copy link
Collaborator

@heckj heckj commented Jan 4, 2025

I may be "jumping the gun" here- but wanted to explore poking at redis locally to work out the middleware using it.

  • this adds a REDIS_HOST environment variable, and references that instead of the string "redis". I've added it to the template for dev and test, and the app.yaml, but I'm not certain about the production ENV variables and what we might want/need to do to set that to "redis" in those locations.
  • adds a redis instance in the makefile and commands to spin up and down
  • adjusts the code in configure to use the environment variable

With this in place, I was able to spin up the local environment on my machine in docker, with an additional local redis instance exposed on 6379 localhost.

@heckj heckj requested a review from finestructure January 4, 2025 21:38
@heckj heckj self-assigned this Jan 4, 2025
@cla-bot cla-bot bot added the cla-signed label Jan 4, 2025
@finestructure
Copy link
Member

Let's hold off on this, Joe. I'm preparing a first "real" use of Redis for our current documentation reference cache. This sets up the infrastructure needed (although it's quite tied to this particular use case for the moment - for example, the nested Redis actor should move to the top level once I'm done testing this setup).

@finestructure
Copy link
Member

Sorry, I just realised I misread the point of this PR, see my comment here: #3582 (comment)

I think all we need to do is weave the Environment.get("REDIS_HOST") into the Redis actor where it's using the hard-coded string right now and then we're good.

app.yml Outdated
@@ -63,6 +63,7 @@ x-shared: &shared
TWITTER_API_SECRET: ${TWITTER_API_SECRET}
TWITTER_ACCESS_TOKEN_KEY: ${TWITTER_ACCESS_TOKEN_KEY}
TWITTER_ACCESS_TOKEN_SECRET: ${TWITTER_ACCESS_TOKEN_SECRET}
REDIS_HOST: ${REDIS_HOST}
Copy link
Member

Choose a reason for hiding this comment

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

Nit: We keep this list in alphabetical order to make it easier to confirm keys are present!

@heckj
Copy link
Collaborator Author

heckj commented Jan 9, 2025

First day I've been able to have any mental capacity to catch back up. Updated this PR to work over the main branch, still leaning into something to make redis available locally for dev work. Actor impl works perfectly, for me, but I wasn't entirely sure how you wanted to handle configuring it dynamically - I thought to reach for an environment variable, following the database host access pieces, but noticed that was also a dependency setup, and wasn't sure if dependencies in dependencies were even a thing, or if a straight processinfo access point inside the actor setup code would be acceptable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants