fix(devservices): Fix healthcheck bug #247
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## main #247 +/- ##
=======================================
Coverage 93.58% 93.58%
=======================================
Files 23 23
Lines 1683 1683
=======================================
Hits 1575 1575
Misses 108 108 ☔ View full report in Codecov by Sentry. |
| @@ -218,7 +218,7 @@ def _up( | |||
| for cmd in up_commands: | |||
| try: | |||
| container_names = get_container_names_for_project( | |||
| cmd.project_name, cmd.config_path | |||
| cmd.project_name, cmd.config_path, cmd.services | |||
There was a problem hiding this comment.
Is there a way to make it clearer that these are the services in the mode?
There was a problem hiding this comment.
Not really besides naming, but I think this should be satisfactory for now, since these are the services that are involved in the docker compose cmd
| @@ -95,7 +95,7 @@ def install_docker_compose() -> None: | |||
|
|
|||
|
|
|||
| def get_container_names_for_project( | |||
| project_name: str, config_path: str | |||
| project_name: str, config_path: str, services: list[str] | |||
There was a problem hiding this comment.
Shouldn't this be a set in theory? I know it would make everything more difficult though
There was a problem hiding this comment.
Yeah, that would be more accurate although this shouldn't be a problem here since container names are always unique
https://linear.app/getsentry/issue/DI-660/healthcheck-bug-for-when-additional-containers-are-already-up