search

cosmos / relayer

An IBC relayer for ibc-go
Apache License 2.0
390 stars 1.71k forks source link

feat: add default entrypoint for the container image #1473

Closed onur-ozkan closed 3 months ago

onur-ozkan commented 5 months ago

I think most developers expect the application to start when running docker run $image instead of terminating the process without doing anything. This PR addresses that expectation.

Reecepbcups commented 5 months ago

I would think developers would expect the relayer binary to run (i.e. help menu) rather than to start. ENTRYPOINT ["rly"] may be more appropriate. Thoughts @onur-ozkan ?

onur-ozkan commented 5 months ago

I am unsure how this will help when we are outside the container. --help(or any command that dumps the command interface) is mostly useful when we are actually using the CLI tool, but in most scenarios when pulling the image you just want to start the application in the container. This is how it's done in other cosmos projects as well like ibc-go and gaia.

onur-ozkan commented 5 months ago

I am unsure how this will help when we are outside the container. --help(or any command that dumps the command interface) is mostly useful when we are actually using the CLI tool, but in most scenarios you just want to start the application in the container. This is how it's done in other cosmos projects as well like ibc-go and gaia.

Oh, seems like ibc-go image does the exact thing you said above.

Reecepbcups commented 5 months ago

@onur-ozkan Yea i think just rly is the best approach. Then adding on arguments as you need them (query, tx, start, etc).

I am going to leave this for someone else to decide on the best approach as this is a breaking change. Thanks for the contribution here! Great thing for us to discuss & add ideally

onur-ozkan commented 5 months ago

this is a breaking change.

Could you please clarify this a bit ? :slightly_smiling_face: At the moment there is no default entry point specified, so people have to specify one manually to make this container to do something useful. By doing so, the entry point we are adding with this PR will be overridden which means they won't notice any difference.

Reecepbcups commented 5 months ago

Sorry, I was thinking docker CMD and not Entrypoint. Entrypoint is not a breaking change. Updated title and marketed out my comment. Thanks!

Reecepbcups commented 4 months ago

Ci failure is due to rate limiting, will re-run this later