Skip to content

orchestra/run: add parameter shell to run()#1639

Closed
rishabh-d-dave wants to merge 1 commit intoceph:masterfrom
rishabh-d-dave:run-add-shell
Closed

orchestra/run: add parameter shell to run()#1639
rishabh-d-dave wants to merge 1 commit intoceph:masterfrom
rishabh-d-dave:run-add-shell

Conversation

@rishabh-d-dave
Copy link
Contributor

Add parameter "shell" to teuthology.orchestra.run.run() with its default
value set to "True" to allow compatiblity with vstart_runner.py and to
also allow programs like vstart_runner.py to exercise greater
flexibility. This should not affect code in teuthology since "shell" is
set to "True" by default for every command executed.

Fixes: https://tracker.ceph.com/issues/50178

# omit_sudo is used by vstart_runner.py
omit_sudo=False
omit_sudo=False,
shell=True
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs docs below.

I wonder if it'd be better to do:

**kwargs

instead so we don't need to keep them in sync all the time?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs docs below.

Added docs.

I wonder if it'd be better to do:

**kwargs

instead so we don't need to keep them in sync all the time?

I personally feel accepting arguments in the current fashion keeps the API easier to get a grip on and also keeps track of parameters that are acceptable.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should stop doing this, like adding useless parameters, and I was hopping we can get rid of omit_sudo as far as we added file operations api with sudo arguments. The vstart_runner.py should not hack teuthology, but it should use it in the correct way.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As argument, the usin of shell=True is a feature already, otherwise multiline script commands will fail once trying to run in this mode.

Add parameter "shell" to teuthology.orchestra.run.run() with its default
value set to "True" to allow compatiblity with vstart_runner.py and to
also allow programs like vstart_runner.py to exercise greater
flexibility. This should not affect code in teuthology since "shell" is
set to "True" by default for every command executed.

Fixes: https://tracker.ceph.com/issues/50178
Signed-off-by: Rishabh Dave <ridave@redhat.com>
@kshtsk
Copy link
Contributor

kshtsk commented May 15, 2021

Sorry guys, we must close this PR.

@batrick
Copy link
Member

batrick commented Jun 11, 2021

@rishabh-d-dave is this PR still fixing the issue cited? I haven't seen that ticket reappear since April.

@rishabh-d-dave
Copy link
Contributor Author

rishabh-d-dave commented Jun 24, 2021

Closing the PR since an alternative solution has been found - ceph/ceph#38481 (comment).

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