-
Notifications
You must be signed in to change notification settings - Fork 73
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
Some configurations/features are ignored if you do not use copy_stack_file
#6
Comments
Thanks for the feedback, pull first + pre deployment args was intended only to be used in the case we copy the file (I will take a look and try to improve this). for the second point, it seems that I forgot to check the input and provide a default value :( . i will update it also |
can
be added when INPUT_COPY_STACK_FILE is false? I cant find a way to pull the latest images otherwise. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Hi there,
first of all thanks for your work here.
I'm using the following configuration for my action
I realized that the input configs
pull_images_first
andpre_deployment_command_args
are completely ignored.Looking at the code it looks to me that both configs will only work if you set the
copy_stack_file
option toTRUE
because you nested the whole logic in this conditionif ! [ -z "$INPUT_COPY_STACK_FILE" ] && [ $INPUT_COPY_STACK_FILE = 'true' ] ; then
Not sure if this is intended or not but for me, there is no reason to make this functionality only available if you copy over the stack file?! It should work as well even without copying the files?
If it was intended, you should maybe update the documentation to reflect this limitation.
Additionally: In some places, you check conditions like this
[ $INPUT_DEPLOYMENT_MODE = 'docker-compose' ]
but if you never setdeployment_mode
explicitly to docker-compose this input var will never have the value as you never set the fallback explicitly.If you say that this behaviour was not intended I'm happy to provide a PR to fix it :)
The text was updated successfully, but these errors were encountered: