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

Resolve ansible-playbook command before execution #492

Merged
merged 2 commits into from
Oct 31, 2023

Conversation

PaulFarault
Copy link
Contributor

Which issue(s) this PR fixes

Fixes None

Additional comments

  • use += operator to compose the command
  • resolve ansible-playbook command before executing the script

Agreements

Split each part on it's own line.
Simplifies the addition of extra_vars.
@PaulFarault PaulFarault requested a review from rpignolet October 31, 2023 10:41
@PaulFarault PaulFarault self-assigned this Oct 31, 2023
@PaulFarault PaulFarault force-pushed the resolve-ansible-playbook-command branch from 98d06c4 to 681a026 Compare October 31, 2023 10:43
@sergkudinov sergkudinov self-requested a review October 31, 2023 11:28
@sergkudinov
Copy link
Contributor

Behavior of tdp deploy is wrong when it is interrupted by the error of missing ansible.

The problem is that the status of the current deployment is changed to RUNNING, but the issue caused this is of a "global" scope, because we shouldn't run the deployment until we have checked all the prerequisites.

To reproduce, first simulate the absence of ansible-playbook in PATH, and then run:

tdp plan dag
tdp deploy --mock-deploy
tdp browse

I am not sure whether it is in the scope of this PR, probably not.

@PaulFarault
Copy link
Contributor Author

PaulFarault commented Oct 31, 2023

In my understanding you are mentioning 2 different issues:

I agree that it is not in the scope of this PR.

@PaulFarault PaulFarault merged commit 4791a94 into master Oct 31, 2023
@PaulFarault PaulFarault deleted the resolve-ansible-playbook-command branch October 31, 2023 14:12
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.

3 participants