Skip to content
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

BREAKING suggestion: change dockerfile to use entrypoint instead of cmd #116

Merged
merged 1 commit into from
Sep 6, 2024

Conversation

samlaf
Copy link
Collaborator

@samlaf samlaf commented Sep 6, 2024

I personally prefer my dockerfiles that are meant to run a single binary to have that binary as the entrypoint. This way I just have to pass the flags directly and don't need to know how the binary was named inside the image.

Compare with this change

docker run eigenda-proxy:latest --memstore.enabled

to what we currently have

docker run eigenda-proxy:latest ./eigenda-proxy --memstore.enabled

This is wordy for no reason, but also I need to inspect Dockerfile manually to check where the binary is located.

This is a breaking change however, given that RaaS's or whoever has this setup in a docker-compose file somewhere will need to change their CMD to work with this (just remove the name of the binary)

@samlaf samlaf requested a review from epociask September 6, 2024 19:37
Copy link
Collaborator

@epociask epociask left a comment

Choose a reason for hiding this comment

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

Nice catch - LGTM!

@samlaf samlaf merged commit 473c880 into main Sep 6, 2024
7 checks passed
@samlaf samlaf deleted the change-dockerfile-cmd-to-entrypoint branch September 6, 2024 20:42
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