almeidajeff commented on code in PR #3681:
URL: https://github.com/apache/ambari/pull/3681#discussion_r1192767128
##########
ambari-agent/conf/unix/agent-multiplier.py:
##########
@@ -226,7 +226,7 @@ def create_host_name_script(self, host_name,
host_name_script):
"echo HOSTNAME"
with open(str(host_name_script), "w+") as f:
f.writelines(template.replace("HOSTNAME", host_name))
- subprocess32.call("chmod +x %s" % host_name_script, shell=True)
+ subprocess.call("chmod +x %s" % host_name_script, shell=True)
Review Comment:
@lingarajg
I believe that the
[subprocess32](https://github.com/apache/ambari/blob/trunk/ambari-common/src/main/python/ambari_commons/subprocess32.py)
module is a copy (maybe there is some customization but I don't know) of
Python's standard subprocess module, so the most viable thing would be to
maintain the use of the module that is already standard in the language and not
maintain a "separate" module, in my opinion it would not make much sense.
As for the use of shell=True, it should be avoided taking into account the
issue of security and the possibility of executing some arbitrary code.
_shell=True_ It is sometimes convenient to make use of shell-specific
features such as word splitting or parameter expansion.
We would have a syntax change at all points where shell=True is executed,
example:
of this
`subprocess.Popen("command -with -options 'like this' and\\ an\\ argument",
shell=True)
`
For that
`subprocess.Popen(['command', '-with','-options', 'like this', 'and an
argument'])`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
