-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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鈥檒l occasionally send you account related emails.
Already on GitHub? Sign in to your account
allow running run-ab-platform.sh in detached mode #26583
Conversation
-b | --background) | ||
dockerDetachedMode="-d" | ||
;; |
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.
@cpdeethree can we have a flag or command to stop the instance? Because today people only will the docker-compose down
anyway. Having this we could update the upgrading docs and everything use the script.
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.
@marcosmarxm I think any command to stop the instance would basically just wrap docker compose down
. I feel like that is an acceptable command to use, since users should know that they are running docker containers via compose. run-ab-platform.sh
is useful for starting the instance because it pulls the correct files/images, but it does not really add anything for stopping the instance.
@@ -155,7 +160,7 @@ done | |||
echo | |||
echo -e "$blue_text""Starting Docker Compose""$default_text" | |||
|
|||
docker compose up | |||
docker compose up $dockerDetachedMode |
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.
I think we need to conditionally call docker compose down
on line 172, right? Currently when I run ./run-ab-platform.sh -b
the containers spin up and then are immediately stopped.
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.
@cpdeethree I'm no bash wizard, but I think this should do it: 19c58f5
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.
@cpdeethree I pushed a commit that appears to be working, would appreciate a quick check, but otherwise LGTM! 馃殺
Looks good! |
* allow running in detached mode * conditionally call docker compose down --------- Co-authored-by: josephkmh <joseph@airbyte.io>
What
Sometimes end users might want to simply start the docker compose process and not retain an open terminal process. This allows them to do that in a single command in the script
How
Add a flag, -b, that allows end users to run in "background" mode. Unfortunately -d was already taken so we use -b instead.
Recommended reading order
馃毃 User Impact 馃毃
No breaking changes