Add option to customize ports for cable and queue #1353

mononoken · 2025-02-02T19:45:55Z

🔗 Issue

n/a

✍️ Description

Right now, our docker-compose.yml has these ports mapped:

  postgres:
    ports:
      - 54320:5432
  ...
  cable:
    ports:
      - 54321:5432
  ...
  queue:
    ports:
      - 54322:5432

However, our database configs do not use these local ports nor have a way to allow their use. This config change would allow a user to set env variables to pick what ports they want locally instead.

One use case would be to set these local ports so that you can run postgresql, cable, and queue in Docker compose but run the app locally with these set:

export DATABASE_PORT=54320
export CABLE_HOST=localhost
export CABLE_PORT=54321
export QUEUE_HOST=localhost
export QUEUE_PORT=54322

I prefer a setup like this, since I like running the database in the Docker container, but I prefer just running the app locally.

This won't affect running the whole app in docker compose, since it keeps the same default ports.

📷 Screenshots/Demos

n/a

kasugaijin

Thanks @mononoken
Should be update the Readme accordingly?

mononoken

@kasugaijin Thanks for taking a look! I noticed the local development without Docker required more configuration than it used to. I fiddled with some other configs so that I think it should be fairly seamless for both use cases. With these changes, if you aren't using docker, you just need to setup your environment variables for your PostgreSQL role, and you should be good to go.

I'm not sure we need to add any comments for the initial change I proposed. We have docs for both Docker and for pure local. I added a little customization to allow someone to do a hybrid approach, but I think that should not be something we document and recommend to do but allow people to do it if they know how to and want to.

Let me know what you think, or if you have any objections or suggestions to any of these changes. Thank you!

mononoken · 2025-02-03T02:20:08Z

config/database.yml

+    host: <%= ENV.fetch("CABLE_HOST") { "localhost" } %>
+    port: <%= ENV.fetch("CABLE_PORT") { "5432" } %>
    migrations_paths: db/cable_migrate
    gssencmode: disable

  queue:
    <<: *default
    database: queue
-    host: <%= ENV.fetch('QUEUE_HOST') { 'queue' } %>
+    host: <%= ENV.fetch("QUEUE_HOST") { "localhost" } %>
+    port: <%= ENV.fetch("QUEUE_PORT") { "5432" } %>


I think these defaults should be optimized for local development without Docker. Docker is another dependency for a dev that they may not know or may not want to use.

That said, I think we should set the hosts to localhost as the default, and we can use the environment variables to set them in the compose for Docker.

mononoken · 2025-02-03T02:20:45Z

docker-compose.yml

+      DATABASE_USERNAME: ${DATABASE_USERNAME:-postgres}
+      DATABASE_PASSWORD: ${DATABASE_PASSWORD:-password}
+      CABLE_HOST: cable
+      QUEUE_HOST: queue


Set defaults for Docker in here instead of in database.yml.

.github/workflows/rubyonrails.yml

config/database.yml

mononoken · 2025-02-03T02:25:29Z

config/database.yml

-  port: <%= ENV.fetch("DATABASE_PORT") do
-          begin
-            docker_port = `docker compose port postgres 5432 2> /dev/null`.split(":").last
-            Integer(docker_port.strip)
-          rescue
-            5432
-          end
-        end %>


I'm not sure why this was necessary at some point, but I think we can get rid of it. Devs should be able to use the default or set an environment variable for customization.

This is for anyone running the app on WSL

I am very skeptical this is actually necessary for WSL. Are you running on Windows? I don't have a Windows machine to test but am fairly confident this new config will work fine for WSL.

jmilljr24

@mononoken I'm on mobile for this week so I can't test anything out but I'm just a bit confused about some of the changes. Forgive me if any of my questions are dumb.

You mentioned the ports in your description but there's no changes to those lines?

For context, in docker-compose.yml, 54321:5432 was required because we are setting up three different containers each with their own postgres database. They each need a different port on your local machine (54321, 54322 etc.) Those ports are mapped to the containers port of 5432.

I don't see a problem with allowing a dev to set a different port but I'm not sure of the use case for postgres running in a container. Nothing else should be running in the container that would conflict with the default and most commonly used port of 5432.

Did you confirm changing the hosts for cable and queue didn't affect running a full docker setup (vs. your hybrid approach). For a new local setup the host variable is being set in application.yml to localhost. If I remember correctly, doing a full docker setup required having the host set different for each container to connect properly.
I just saw how you switched it. That makes sense to have local development as the default and then define the different hosts for docker compose!

config/database.yml

mononoken · 2025-02-04T05:36:16Z

@kasugaijin I made some updates to the README. Let me know if you have any other suggestions. I also opened #1358. We will need to merge that first for the Standard job to run on this branch. Hoping to make tomorrow's meeting too if you would prefer to discuss any of this real time.

jmilljr24 · 2025-02-04T12:21:15Z

.github/workflows/rubyonrails.yml

+      DATABASE_USERNAME: rails
+      DATABASE_PASSWORD: password


I merged the workflow changes but the "standard" job is failing. We might need to add these under "standard" in this workflow as well.

Thanks, @jmilljr24 !

That seems suspect. Why is the standard job accessing the db at all? Looks like it is because it is using the Rails runner: https://github.com/rubyforgood/homeward-tails/blob/9c2652599da400799ed6751d136776da382762d4/bin/style#L7C1-L7C19

I think it could just run with bundler:

bundle exec standardrb

I can take a further look tonight.

Good question. I took a quick look at the docs for Standard but nothing stood out. If you get more insight I'd love to know!

Opened #1360. Hopefully that will get this ready to merge too.

…ker usecase

mononoken · 2025-02-05T05:17:00Z

Rebased to remove the workflow changes from this one, since those changes are in separate PRs now. I also did a little more cleaning up the README. I considered moving Docker below the local install stuff, but since it's a lot less steps to just download Docker and use docker compose, I think it makes sense to keep it up top.

jmilljr24

@mononoken lgtm

kasugaijin

LGTM.

mononoken requested review from jmilljr24 and kasugaijin February 2, 2025 19:47

mononoken marked this pull request as ready for review February 2, 2025 19:50

kasugaijin reviewed Feb 2, 2025

View reviewed changes

mononoken commented Feb 3, 2025

View reviewed changes

jmilljr24 reviewed Feb 3, 2025

View reviewed changes

mononoken commented Feb 3, 2025

View reviewed changes

config/database.yml Show resolved Hide resolved

mononoken mentioned this pull request Feb 4, 2025

Update gh workflow env variables #1358

Merged

jmilljr24 reviewed Feb 4, 2025

View reviewed changes

mononoken force-pushed the fix-local-docker-dbs branch from 062dc1f to 3fef61d Compare February 5, 2025 04:50

mononoken added 16 commits February 4, 2025 20:50

Add option to customize ports for cable and queue

7c07e04

Remove default postgres credentials

ac27687

Remove overcomplicated block

bc945a1

Remove empty blocks

ce326fa

Set defaults for cable and queue to localhost

6508a7f

Add host variables to compose

065f690

Remove unused comments and lint

869a926

Set default database username and password in compose instead for doc…

ec53f1f

…ker usecase

Use env variable for cable and queue database init too

76d88d5

Unify formatting

169e754

Use port variables for compose too

b2fd726

Specify PostgreSQL version for brew

3801390

These are not necessary anymore

a9f88ce

Update postgresql instructions

c2388cf

Remove line no longer necessary with new config

d9ada46

Move Docker instructions below local

3ad20d3

mononoken force-pushed the fix-local-docker-dbs branch from 3fef61d to 3ad20d3 Compare February 5, 2025 04:50

Cleanup README more

9fe93c1

mononoken requested review from jmilljr24 and kasugaijin February 7, 2025 04:24

jmilljr24 approved these changes Feb 7, 2025

View reviewed changes

kasugaijin approved these changes Feb 8, 2025

View reviewed changes

kasugaijin merged commit 2a86c7a into main Feb 8, 2025
5 checks passed

kasugaijin deleted the fix-local-docker-dbs branch February 8, 2025 04:10

jmilljr24 mentioned this pull request Feb 9, 2025

Docker Compose Sass Error #1362

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add option to customize ports for cable and queue #1353

Add option to customize ports for cable and queue #1353

mononoken commented Feb 2, 2025 •

edited

Loading

kasugaijin left a comment

mononoken left a comment

mononoken Feb 3, 2025

mononoken Feb 3, 2025

mononoken Feb 3, 2025

kasugaijin Feb 3, 2025

mononoken Feb 4, 2025

jmilljr24 left a comment •

edited

Loading

mononoken commented Feb 4, 2025

jmilljr24 Feb 4, 2025

mononoken Feb 4, 2025 •

edited

Loading

jmilljr24 Feb 4, 2025

mononoken Feb 5, 2025

mononoken commented Feb 5, 2025

jmilljr24 left a comment

kasugaijin left a comment

Add option to customize ports for cable and queue #1353

Add option to customize ports for cable and queue #1353

Conversation

mononoken commented Feb 2, 2025 • edited Loading

🔗 Issue

✍️ Description

📷 Screenshots/Demos

kasugaijin left a comment

Choose a reason for hiding this comment

mononoken left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jmilljr24 left a comment • edited Loading

Choose a reason for hiding this comment

mononoken commented Feb 4, 2025

Choose a reason for hiding this comment

mononoken Feb 4, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mononoken commented Feb 5, 2025

jmilljr24 left a comment

Choose a reason for hiding this comment

kasugaijin left a comment

Choose a reason for hiding this comment

mononoken commented Feb 2, 2025 •

edited

Loading

jmilljr24 left a comment •

edited

Loading

mononoken Feb 4, 2025 •

edited

Loading