Skip to content

Conversation

@dkoslicki
Copy link
Collaborator

But I barely know what I am doing as this is my first time working with Docker compose

… my first time working with Docker compose
@dkoslicki dkoslicki requested a review from maximusunc June 4, 2025 16:33
@codecov
Copy link

codecov bot commented Jun 4, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 45.26%. Comparing base (457ec71) to head (4faad35).
Report is 7 commits behind head on main.

Files with missing lines Coverage Δ
shepherd_utils/config.py 100.00% <100.00%> (ø)

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8ca1cf2...4faad35. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Collaborator

@maximusunc maximusunc left a comment

Choose a reason for hiding this comment

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

That's awesome that this fixes it! It seems like the issue is that on your local machine inside of your docker, the individual containers aren't able to resolve localhost to their local docker network (I've seen issues like this before and it's a crapshoot and based on computer). I suggest we make this change in one location here: https://github.com/BioPack-team/shepherd/blob/main/shepherd_utils/config.py which is where those urls are defined for all services. And I think using shepherd_db and shepherd_broker is the right approach. Docker will know how to DNS those names and our local development is going to use docker fairly exclusively. One could technically run everything local, but it would require spinning up your own redis, postgres, any other dbs you need, and then run each worker as a separate python process, which is going to be a lot of work.

@dkoslicki
Copy link
Collaborator Author

How's that look? I may have mixed up which was the broker (as they're named in that config redis and postgres)

Copy link
Collaborator

@maximusunc maximusunc left a comment

Choose a reason for hiding this comment

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

Yep, looks great!

@maximusunc maximusunc merged commit ac5874d into main Jun 4, 2025
3 checks passed
@maximusunc maximusunc deleted the issue21 branch June 4, 2025 19:17
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.

3 participants