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

Optional --run-directory #488

Closed
PaulFarault opened this issue Oct 30, 2023 · 3 comments · May be fixed by #612
Closed

Optional --run-directory #488

PaulFarault opened this issue Oct 30, 2023 · 3 comments · May be fixed by #612
Assignees
Labels
question Further information is requested

Comments

@PaulFarault
Copy link
Contributor

On the tdp deploy command, the --run-directory option is used to define the working directory when spawning a subprocess to execute an Ansible command (using subprocess.Popen). If no directory is provided, the subprocess uses the parent process directory.

We made both the --run-directory option mandatory on the cli AND the run_directory parameter optional on the core. Should we keep this behavior, ensure that the working directory is provided or make it entirely optional?


If we choose to keep this behavior, we should at least remove the following condition in the deploy command (as the run_directory is required):

Executor(
    run_directory=run_directory.absolute() if run_directory else None,
    # ...
),
@PaulFarault PaulFarault added the question Further information is requested label Oct 30, 2023
@PaulFarault PaulFarault mentioned this issue Oct 30, 2023
2 tasks
@Pierrotws
Copy link

Having it mandatory doesn't make sense imho.

Also, the standard name for this is cwd (current working directory), that's what I would choose as name.

--run-directory is way too long.

@Pierrotws
Copy link

That's what is chosen by yarn (package manager) :

yarn --cwd ~/test_project/ dev

@PaulFarault
Copy link
Contributor Author

Make sense, thanks! I'll make a PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants