# Is it a good style to run external commands in Python? [closed]

I have lots of external shell commands to run.

So I gave every command a name and then run it.

tar_report_cmd = 'zip -r  {0} {1} '%(report_file, _export_dir)
exec_proc(tar_report_cmd)
mv_report_cmd = "mv {0} {1}".format(report_file,report_dir)
exec_proc(mv_report_cmd)


But it looks sort of redundant and not beautiful.

Is there any better practice to replace my way?

def exec_proc(cmd_str):
return results


## closed as off-topic by Stephen Rauch, Daniel, Zeta, Billal Begueradj, GraipherJul 6 '18 at 9:05

This question appears to be off-topic. The users who voted to close gave this specific reason:

• "Lacks concrete context: Code Review requires concrete code from a project, with sufficient context for reviewers to understand how that code is used. Pseudocode, stub code, hypothetical code, obfuscated code, and generic best practices are outside the scope of this site." – Stephen Rauch, Daniel, Zeta, Billal Begueradj, Graipher
If this question can be reworded to fit the rules in the help center, please edit the question.

• For the examples given, I would suggest using the library support for zip files and moving files. It's more cross-platform and gives you better precision and error recovery options. Tend towards calling other commands only when library versions are unavailable and do exactly what you need. – Michael Urman Feb 18 '14 at 13:20
• This question appears to be off-topic because it is primarily about how to write code, rather than really reviewing code that has been written. – Michael Urman Feb 18 '14 at 13:21
• @MichaelUrman The code may be written at a novice level, but it's still reviewable code. We're all here to learn how to write better code, and this question is no different. – 200_success Feb 18 '14 at 23:38
• @MichaelUrman Also, your comment that this should be done using a Python library rather than an external command should be posted as an answer, since it is a suggestion that would result in an improvement. – 200_success Feb 18 '14 at 23:38
• @200_success I agree that if it were an on-topic question, my comment should be posted as an answer. But I see this as asking if it's a good approach, rather than asking for a code review. Note that the code given doesn't even work (what's report_file, _export_dir, ...?). – Michael Urman Feb 19 '14 at 2:50

No, it is a security risk, and furthermore, os.popen() has been deprecated since Python 2.6. The replacement is to use the subprocess module.
The security risk is that os.popen() expects its command as a string. That means that the command will be interpreted by a shell to split it up into the command and its arguments. If report_file, report_dir, or _export_dir contain any spaces or characters with a special meaning to the shell, horrible unexpected things can happen, including arbitrary command execution.