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

Add environment key ENV #458

Merged
merged 4 commits into from
Mar 20, 2018
Merged

Add environment key ENV #458

merged 4 commits into from
Mar 20, 2018

Conversation

twhiston
Copy link
Contributor

This adds the optional key ENV to the environment that allows you to pass command execution level arguments to the run.
This is useful if you need to alter something such as the ansible.cfg path, which needs to be done via env var due to the working directory for semaphore being set to the playbook/tmp path.
This would also satisfy the final request of the (closed issue) #222

I'm not sure if this is the way that people want to go with this sort of feature, but having a special key in the environment json seemed like the lowest cost way to achieve this without db migrations and ui updates etc.... If people feel it muddies the waters regarding the purpose of the environment file then I will happily implement it in a different way

This adds the optional key ENV to the environment that allows you to pass command execution level arguments to the run
@twhiston
Copy link
Contributor Author

@strangeman any thoughts about this? I think we should consider adding a way to pass environmental variables to the command execution environment, rather than just variables to ansible, but this is probably not the way to do it! Should we add another field to the task/task run dialogs that allow you to specify environment variables? Any better ideas?

@strangeman
Copy link
Contributor

Uh, this is the strange feature. I worked more than 3 years with Ansible, in different environments and never used external ENV variables for it. In my personal opinion, this is a rarely used feature, so we may implement it in simple (and a bit dirty) way.

@twhiston
Copy link
Contributor Author

without this feature how would you pass a custom ansible.cfg file? putting them in the project root is not a good enough solution as you may have multiple playbooks in a single repo, subfolders etc.... this is the specific use case i did this for as it's something we needed at work. I would be very open to a better solution though 😀

@strangeman
Copy link
Contributor

Hmm... we just store it in the project root. :) We have multiple playbooks, but ansible.cfg contains only basic information. Some variables (like remote_user) overridden via variables, another - via flags.

@twhiston
Copy link
Contributor Author

we have a specific use case then, as we store all our openshift cluster version playbooks in the same repo and have to use different ansible.cfg files to pass distinct plugin folder locations for each.
So would you support doing it this way for now (and an addition to the docs clearly explaining it, maybe also a ui addition too?) or shall i think about a more robust implementation?

@strangeman
Copy link
Contributor

Let's implement it as is. If the users will request the more clean way to pass ENV variables - we'll make another task.

@semaphoreui semaphoreui deleted a comment Mar 14, 2018
@semaphoreui semaphoreui deleted a comment Mar 14, 2018
@semaphoreui semaphoreui deleted a comment Mar 14, 2018
@twhiston
Copy link
Contributor Author

ok, i've additionally fixed the issues from codacy and added to the environment interface text to mention the ENV key. I will probably also add something in the wiki in future but rewriting that is a separate task I think.

@twhiston
Copy link
Contributor Author

@strangeman, can you give this a quick once over and if you are happy I will merge it

@twhiston twhiston merged commit d22f2eb into semaphoreui:develop Mar 20, 2018
@twhiston twhiston deleted the cmd_environment_vars branch March 20, 2018 00:33
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