-
Notifications
You must be signed in to change notification settings - Fork 25
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[#357] Fix: docker compose command is not working as expected #504
Merged
Conversation
This file contains 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
sanG-github
force-pushed
the
bug/gh357-compose-v2
branch
12 times, most recently
from
January 30, 2024 08:28
32a54e3
to
0bd40bf
Compare
sanG-github
force-pushed
the
bug/gh357-compose-v2
branch
3 times, most recently
from
March 13, 2024 07:51
aad4236
to
18c7581
Compare
debug debug Add log
Update container name Update container name
sanG-github
force-pushed
the
bug/gh357-compose-v2
branch
from
March 13, 2024 08:11
a5b5499
to
1be7c94
Compare
sanG-github
requested review from
aetheryna,
andyduong1920,
bterone and
byhbt
as code owners
March 13, 2024 10:28
sanG-github
requested review from
carryall,
Goose97,
hoangmirs,
longnd,
malparty,
vnntsu and
tyrro
as code owners
March 13, 2024 10:28
malparty
approved these changes
Apr 9, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just not sure about the last usage of APP_NAME
👀
khangbui22
reviewed
Apr 9, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🚀
khangbui22
approved these changes
Apr 9, 2024
Merged
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.
What happened 👀
Update the
docker-compose
from v1 todocker compose
v2. 🚀Update the
APP_NAME
to usehyphen
instead ofunderscore
when it comes tocontainer_name
.Insight 📝
When changing the docker-compose from version 1 to version 2, the gist is that they keep the container name to be a valid name for DNS hostnames with
hyphen (-)
as a separator instead ofunderscore (_)
as before. (ref)Note
With the APP_NAME =
rails_template
For the current codebase, we previously used
underscore
for all the container names primarily, such as:rails_template_test_run_9ae09012e3d0
rails_template_redis
But after changing docker-compose to a newer version without any additional modifications, those container names become:
rails_template-test-run-9ae09012e3d0
rails_template-redis
-> So we need to update all container names to be consistent with that convention, using only
hyphen
.Caution
We previously tried to find the
test run
container by the name, but it was used withunderscore
-> challenge when switching to usehyphen
as it keeps raising the container name not found.I tried to log some information when the test flow was running, but it looked weird.
In this workflow run, the log returns a list of the container name and ID respectively. And we can get the
ID
from thecontainer name
as show below:But after changing it to
hyphen
, with the same approach, we cannot retrieve that container ID in this workflow run.Proof Of Work 📹