[keyserver] Make it possible to configure Redis host
ClosedPublic
Actions

Authored by ashoat on Apr 25 2022, 7:49 PM.

Details

Reviewers

tomek
• jim

Commits

rCOMM2847cecdf3ea: [keyserver] Make it possible to configure Redis host

Summary

Today we always hit localhost. I'm working on Dockerizing the keyserver, and since we're using docker-compose we will need to allow this to be configurable.

Depends on D3849

Test Plan

in concert with D3851, I tested that yarn update-geoip worked inside the Docker image (it accesses Redis, admittedly for no good reason)

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

ashoat created this revision.Apr 25 2022, 7:49 PM

Herald added subscribers: • abosh, • karol, atul, • adrian. · View Herald TranscriptApr 25 2022, 7:49 PM

ashoat added a child revision: D3851: [keyserver] Add Redis to docker-compose.yml.Apr 25 2022, 7:49 PM

Harbormaster completed remote builds in B8538: Diff 11900.Apr 25 2022, 7:59 PM

ashoat requested review of this revision.Apr 25 2022, 7:59 PM

tomek accepted this revision.Apr 26 2022, 9:46 AM

tomek added inline comments.

keyserver/src/socket/redis.js
24–31 ↗	(On Diff #11900)	What is the reason for using immediately invoked function?

This revision is now accepted and ready to land.Apr 26 2022, 9:46 AM

ashoat added inline comments.Apr 27 2022, 11:37 AM

keyserver/src/socket/redis.js
24–31 ↗	(On Diff #11900)	Using a ternary like you suggested would break my third rule of ternary conditionals: Avoid ternary conditionals that don’t format cleanly Ternary conditionals should be obvious at-a-glance. Either the whole expression should fit on one line, or the `?` and `:` characters should appear at the start of subsequent lines. I could've alternately used a `let` declaration, but I prefer to avoid those since I think it's harder to tell what the variable's value will be (you have to search to see if it's reassigned anywhere)