Skip to content

Run shell as default if no arguments are given upon docker run#1

Merged
mlsdpk merged 2 commits intomlsdpk:masterfrom
ghanta1996:master
Mar 1, 2025
Merged

Run shell as default if no arguments are given upon docker run#1
mlsdpk merged 2 commits intomlsdpk:masterfrom
ghanta1996:master

Conversation

@ghanta1996
Copy link
Copy Markdown
Contributor

Added exec bash at the end of the entrypoint.sh. This will help docker run command to run open bash

@mlsdpk mlsdpk self-requested a review February 23, 2025 18:49
Copy link
Copy Markdown
Owner

@mlsdpk mlsdpk left a comment

Choose a reason for hiding this comment

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

Thanks for this fix! Just minor changes need to be done and we're good to go. Please also update the changelog to reflect this update.

entrypoint.sh Outdated
source /venv/bin/activate
exec "$@" No newline at end of file
exec "$@"
exec bash No newline at end of file
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I think this line exec bash will not be executed if the user provides custom commands. May be we could add a conditional statement to execute /bin/bash only if no command is provided so that we can still support both options (allow the container to default to an interactive shell)

@ghanta1996 ghanta1996 requested a review from mlsdpk March 1, 2025 12:03
Copy link
Copy Markdown
Owner

@mlsdpk mlsdpk left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks

@mlsdpk mlsdpk merged commit fe52a60 into mlsdpk:master Mar 1, 2025
2 checks passed
@mlsdpk mlsdpk changed the title fix: add exec bash to make the entrypoint.sh run Run shell as default if no arguments are given upon docker run Mar 1, 2025
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.

2 participants