Added a sudo argument to cli.Client.run to avoid boilerplate.
#1132
+62
−1
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rochacbruno
force-pushed
the
add_sudo_function
branch
2 times, most recently
from
592a0d6 to
a5dcf46
Compare
nixocio
reviewed
Sep 5, 2018
pulp_smash/cli.py
Outdated
| which prior to execution checks if user is root and if not | ||
| adds ``sudo`` as the first argument of the command. | ||
| """ | ||
| if args[0] != 'sudo' and not is_root(self.cfg): |
There was a problem hiding this comment.
@rochacbruno, thanks for this PR.
I have seen this idiom being used.
sudo = '' if cli.is_root(cls.cfg) else 'sudo 'I have not looked into details your PR, but about the space after sudo like the example above. Did you cover this case as well?
There was a problem hiding this comment.
@kersommoura The space there is because the use of .split() on a string. As run will accept only a list/tuple it is not a problem, we can replace those calls with just client.sudo.
omaciel
reviewed
Sep 5, 2018
There was a problem hiding this comment.
What if the run method accepted an optional sudo flag instead? To me, client.run() is very intuitive, and so is client.sudo() but since they both do the same action but with different "permissions", we could have the same result if we were to change the existing method to something like:
client.run(('ls', '-l'), sudo=True)Thoughts?
As part of our `less boilerplate` effort I added a new method `sudo` argument
to `cli.Client.run`.
In a lot of test cases there are code like:
```python
sudo = () if cli.is_root(cfg) else ('sudo',)
...
client.run(sudo + ('command_name', 'argument1', ...))
```
The idea is to replace the above boilerplate with
```python
client.run(('command_name', 'argument1', ...), sudo=True)
```
With this change we will be able to remove the `sudo = () if cli.is_root(cfg) else ('sudo',)` boilerplate in those Pulp-2-tests referenced:
```bash
pulp_2_tests/tests/docker/utils.py: sudo = '' if cli.is_root(cfg) else 'sudo'
pulp_2_tests/tests/puppet/api_v2/test_install_distributor.py: sudo = () if cli.is_root(self.cfg) else ('sudo',)
pulp_2_tests/tests/rpm/api_v2/test_content_sources.py: sudo = () if cli.is_root(cls.cfg) else ('sudo',)
pulp_2_tests/tests/rpm/api_v2/test_content_sources.py: sudo = () if cli.is_root(cls.cfg) else ('sudo',)
pulp_2_tests/tests/rpm/api_v2/test_content_sources.py: sudo = '' if cli.is_root(cfg) else 'sudo'
pulp_2_tests/tests/rpm/api_v2/test_crud.py: sudo = () if cli.is_root(self.cfg) else ('sudo',)
pulp_2_tests/tests/rpm/api_v2/test_download_policies.py: sudo = '' if cli.is_root(cls.cfg) else 'sudo '
pulp_2_tests/tests/rpm/api_v2/test_errata.py: # sudo = () if cli.is_root(cfg) else ('sudo',)
pulp_2_tests/tests/rpm/api_v2/test_errata.py: sudo = () if cli.is_root(cfg) else ('sudo',)
pulp_2_tests/tests/rpm/api_v2/test_export.py: self.__sudo = None
pulp_2_tests/tests/rpm/api_v2/test_export.py: self.__sudo = '' if cli.is_root(self.cfg) else 'sudo '
pulp_2_tests/tests/rpm/api_v2/test_iso_sync_publish.py: sudo = () if cli.is_root(self.cfg) else ('sudo',)
pulp_2_tests/tests/rpm/api_v2/test_modularity.py: sudo = () if cli.is_root(cfg) else ('sudo',)
pulp_2_tests/tests/rpm/api_v2/test_republish.py: sudo = () if cli.is_root(self.cfg) else ('sudo',)
pulp_2_tests/tests/rpm/api_v2/test_rich_weak_dependencies.py: sudo = () if cli.is_root(cfg) else ('sudo',)
pulp_2_tests/tests/rpm/api_v2/test_rsync_distributor.py: sudo = () if cli.is_root(cfg) else ('sudo',)
pulp_2_tests/tests/rpm/api_v2/test_rsync_distributor.py: sudo = () if cli.is_root(cfg) else ('sudo',)
pulp_2_tests/tests/rpm/api_v2/test_rsync_distributor.py: sudo = '' if cli.is_root(cfg) else 'sudo '
pulp_2_tests/tests/rpm/api_v2/test_service_resiliency.py: sudo = '' if cli.is_root(self.cfg) else 'sudo'
pulp_2_tests/tests/rpm/api_v2/test_service_resiliency.py: sudo = () if cli.is_root(self.cfg) else ('sudo',)
pulp_2_tests/tests/rpm/api_v2/utils.py: sudo = '' if cli.is_root(cfg) else 'sudo '
pulp_2_tests/tests/rpm/api_v2/utils.py: sudo = '' if cli.is_root(cfg) else 'sudo '
pulp_2_tests/tests/rpm/api_v2/utils.py: sudo = () if cli.is_root(cfg) else ('sudo',)
pulp_2_tests/tests/rpm/api_v2/utils.py: sudo = '' if cli.is_root(cfg) else 'sudo '
pulp_2_tests/tests/rpm/api_v2/utils.py: sudo = () if cli.is_root(cfg) else ('sudo',)
pulp_2_tests/tests/rpm/cli/test_copy_units.py: sudo = () if cli.is_root(cfg) else ('sudo',)
pulp_2_tests/tests/rpm/cli/test_process_recycling.py: sudo = () if cli.is_root(cfg) else ('sudo',)
pulp_2_tests/tests/rpm/cli/test_process_recycling.py: sudo = () if cli.is_root(cfg) else ('sudo',)
```
rochacbruno
force-pushed
the
add_sudo_function
branch
from
a5dcf46 to
f8a21c3
Compare
rochacbruno
changed the title
sudo method to cli.Client to avoid boilerplate.sudo argument to cli.Client.run to avoid boilerplate.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fix #1037
As part of our
less boilerplateeffort I added a new methodsudoargumentto
cli.Client.run.In a lot of test cases there are code like:
The idea is to replace the above boilerplate with
With this change we will be able to remove the
sudo = () if cli.is_root(cfg) else ('sudo',)boilerplate in 28 Pulp-2-tests references: