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.

Sign up for GitHub

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

Jump to bottom

Provide NULL instead of empty string, when Extra CLI Arguments was deleted #286

Merged

matejkramny merged 1 commit into semaphoreui:master from UnitedTraders:empty-cli-args

Mar 13, 2017

Contributor

strangeman commented Mar 10, 2017

Fix for #171


          provide NULL instead of empty string, when Extra CLI Arguments was de…

…leted

Contributor

matejkramny commented Mar 10, 2017

What if template.Arguments is nil, in which case this might cause a panic?

Contributor Author

strangeman commented Mar 11, 2017

@matejkramny it shouldn't. We use template.Arguments in two places only, as I see:

We add/edit template.Arguments in https://github.com/ansible-semaphore/semaphore/blob/master/api/projects/templates.go#L60 and in https://github.com/ansible-semaphore/semaphore/blob/master/api/projects/templates.go#L94, and MySQL correctly inserts NULL value in this case.
We pass template.Arguments to the ansible-playbook command in https://github.com/ansible-semaphore/semaphore/blob/master/api/tasks/runner.go#L310 and checking if it not nil.

Contributor Author

strangeman commented Mar 11, 2017 •

edited

Loading

BTW, we may use another solution:

In https://github.com/ansible-semaphore/semaphore/blob/master/api/tasks/runner.go#L310:

- 	if t.template.Arguments != nil {
+ 	if t.template.Arguments != nil && *t.template.Arguments != "" {

matejkramny merged commit 37c6d17 into semaphoreui:master

strangeman deleted the empty-cli-args branch

March 13, 2017 00:58

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet