fix(up): add Service.containerName helper for explicit container_name#113
Open
YvanY0 wants to merge 1 commit into
Open
fix(up): add Service.containerName helper for explicit container_name#113YvanY0 wants to merge 1 commit into
YvanY0 wants to merge 1 commit into
Conversation
ComposeUp had three places (getIPForRunningService, waitUntilServiceIsRunning, stopOldStuff) that hard-coded the container name as "\(projectName)-\(serviceName)" instead of reading service.container_name. When a compose file set an explicit container_name, the container started correctly (container run --name used the right value), but the post-start lookups waited for the wrong name and timed out after 30s. Extract a single Service.containerName(projectName:serviceName:) helper and replace all five call sites (four in ComposeUp, one in ComposeDown) with it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The bug
When a compose file sets an explicit
container_name, the container starts correctly —container run --namereads the value. ButComposeUphas three other places that look up the running container by name (waitUntilServiceIsRunning,getIPForRunningService,stopOldStuff), and all three hard-code"\(projectName)-\(serviceName)"instead of readingcontainer_name.For example, given:
Run from a directory named
hermes(noname:field), the container starts ashermesbut the post-start poll waits forhermes-hermesand times out:Issue #38 reported the same bug on the
downside; PR #39 fixedComposeDown.stopOldStuffbutComposeUpwas not addressed.The fix
Extract
Service.containerName(projectName:serviceName:)— a single method that encapsulates the rule (prefer explicitcontainer_name, fall back to"\(projectName)-\(serviceName)") — and route all five call sites through it:configService,waitUntilServiceIsRunning,getIPForRunningService,stopOldStuff)stopOldStuffin ComposeDown (refactored to use the same helper, replacing the inlineif/elsefrom PR Handle compose down with container_name case #39)When
container_nameis not set the helper returns the same"\(projectName)-\(serviceName)"string as before; existing compose files without explicit container names are unaffected.Verification
Tested locally against the compose file above:
Before:
container-compose up -dreportsTimed out waiting for container 'hermes-hermes' to be running.After: the container starts and polls successfully with no timeout;
container ls -ashows it inrunningstate.Tests
ServiceNamingTests— two unit tests for the purecontainerNamehelper (explicit name wins, default pattern when unset).testUpWithExplicitContainerName— exercisesComposeUp.run()end-to-end with an explicitcontainer_name(mirrors the pattern in PR feat: Providing tests forComposeDown#50'stestUpAndDownContainerName).Fixes #38