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

Python2 codepath is incorrect as check_call() does NOT capture output. #33

Conversation

ShaheedHaque
Copy link

@ShaheedHaque ShaheedHaque commented Jul 2, 2018

Also, the Python2/3 exception handling path is suboptimal as firstly,
logger.exception() outputs the exception info already (since exc_info is
True by default), and secondly, in both cases it has access to any output
generated which may as well be the msg argument.

(I note that my previous PR#28 didn't address this wider point). For example,
without the fix, the logged output is something like this:

[2018-07-02 09:14:55,059: ERROR/ForkPoolWorker-15] Command '/usr/local/lib/python3.6/dist-packages/pyreportjasper/jasperstarter/bin/jasperstarter --locale en_GB process "/main/srhaque/Innovatieltd/testdjango/paiyroll/report/JasperReports/pre_approval.jrxml" -o "/tmp/Acme Accountants/Advanced Mechanics/m1/2018-07-28/pre_approval" -f pdf -t jsonql --data-file '/tmp/Acme Accountants/Advanced Mechanics/m1/2018-07-28/snapshot.json' --jsonql-query 'employee.'' returned non-zero exit status 1.
Traceback (most recent call last):
File "/usr/local/lib/python3.6/dist-packages/pyreportjasper/jasperpy.py", line 188, in execute
self.command, shell=True, check=True).returncode
File "/usr/lib/python3.6/subprocess.py", line 418, in run
output=stdout, stderr=stderr)
subprocess.CalledProcessError: Command '/usr/local/lib/python3.6/dist-packages/pyreportjasper/jasperstarter/bin/jasperstarter --locale en_GB process "/main/srhaque/Innovatieltd/testdjango/paiyroll/report/JasperReports/pre_approval.jrxml" -o "/tmp/Acme Accountants/Advanced Mechanics/m1/2018-07-28/pre_approval" -f pdf -t jsonql --data-file '/tmp/Acme Accountants/Advanced Mechanics/m1/2018-07-28/snapshot.json' --jsonql-query 'employee.
'' returned non-zero exit status 1.
[2018-07-02 09:14:55,062: ERROR/ForkPoolWorker-15] Unhandled exception
Traceback (most recent call last):
File "/usr/local/lib/python3.6/dist-packages/pyreportjasper/jasperpy.py", line 188, in execute
self.command, shell=True, check=True).returncode
File "/usr/lib/python3.6/subprocess.py", line 418, in run
output=stdout, stderr=stderr)
subprocess.CalledProcessError: Command '/usr/local/lib/python3.6/dist-packages/pyreportjasper/jasperstarter/bin/jasperstarter --locale en_GB process "/main/srhaque/Innovatieltd/testdjango/paiyroll/report/JasperReports/pre_approval.jrxml" -o "/tmp/Acme Accountants/Advanced Mechanics/m1/2018-07-28/pre_approval" -f pdf -t jsonql --data-file '/tmp/Acme Accountants/Advanced Mechanics/m1/2018-07-28/snapshot.json' --jsonql-query 'employee.*'' returned non-zero exit status 1.

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
...
File "/usr/local/lib/python3.6/dist-packages/pyreportjasper/jasperpy.py", line 151, in process
return self.execute()
File "/usr/local/lib/python3.6/dist-packages/pyreportjasper/jasperpy.py", line 193, in execute
raise NameError('Your report has an error and couldn '
NameError: Your report has an error and couldn 't be processed!\ Try to output the command using the attribute command; and run it manually in the console!

(Notice how the word "Command" appears 3 times). With the fix the output is the less redundant:

[2018-07-02 09:06:44,706: ERROR/ForkPoolWorker-2] None
Traceback (most recent call last):
File "/usr/local/lib/python3.6/dist-packages/pyreportjasper/jasperpy.py", line 188, in execute
self.command, shell=True, check=True).returncode
File "/usr/lib/python3.6/subprocess.py", line 418, in run
output=stdout, stderr=stderr)
subprocess.CalledProcessError: Command '/usr/local/lib/python3.6/dist-packages/pyreportjasper/jasperstarter/bin/jasperstarter --locale en_GB process "/main/srhaque/Innovatieltd/testdjango/paiyroll/report/JasperReports/pre_approval.jrxml" -o "/tmp/Acme Accountants/Advanced Mechanics/m1/2018-07-28/pre_approval" -f pdf -t jsonql --data-file '/tmp/Acme Accountants/Advanced Mechanics/m1/2018-07-28/snapshot.json' --jsonql-query 'employee.'' returned non-zero exit status 1.
[2018-07-02 09:06:44,708: ERROR/ForkPoolWorker-2] Unhandled exception
Traceback (most recent call last):
File "/usr/local/lib/python3.6/dist-packages/pyreportjasper/jasperpy.py", line 188, in execute
self.command, shell=True, check=True).returncode
File "/usr/lib/python3.6/subprocess.py", line 418, in run
output=stdout, stderr=stderr)
subprocess.CalledProcessError: Command '/usr/local/lib/python3.6/dist-packages/pyreportjasper/jasperstarter/bin/jasperstarter --locale en_GB process "/main/srhaque/Innovatieltd/testdjango/paiyroll/report/JasperReports/pre_approval.jrxml" -o "/tmp/Acme Accountants/Advanced Mechanics/m1/2018-07-28/pre_approval" -f pdf -t jsonql --data-file '/tmp/Acme Accountants/Advanced Mechanics/m1/2018-07-28/snapshot.json' --jsonql-query 'employee.
'' returned non-zero exit status 1.

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
...
File "/usr/local/lib/python3.6/dist-packages/pyreportjasper/jasperpy.py", line 151, in process
return self.execute()
File "/usr/local/lib/python3.6/dist-packages/pyreportjasper/jasperpy.py", line 193, in execute
raise NameError('Your report has an error and couldn '
NameError: Your report has an error and couldn 't be processed!\ Try to output the command using the attribute command; and run it manually in the console!

(Notice how the first "Command" has been replaced by "None", and would have been
replaced by any output from the command for both Python2/3).

Also, the Python2/3 exception handling path is suboptimal as firstly,
logger.exception() outputs the exception info already (since exc_info is
True by default), and secondly, in both cases it has access to any output
generated which may as well be the msg argument.

(I note that my previous PR#28 didn't address this wider point). For example,
without the fix, the logged output is something like this:

=========
[2018-07-02 09:14:55,059: ERROR/ForkPoolWorker-15] Command '/usr/local/lib/python3.6/dist-packages/pyreportjasper/jasperstarter/bin/jasperstarter --locale en_GB process "/main/srhaque/Innovatieltd/testdjango/paiyroll/report/JasperReports/pre_approval.jrxml" -o "/tmp/Acme Accountants/Advanced Mechanics/m1/2018-07-28/pre_approval" -f pdf -t jsonql --data-file '/tmp/Acme Accountants/Advanced Mechanics/m1/2018-07-28/snapshot.json' --jsonql-query 'employee.*'' returned non-zero exit status 1.
Traceback (most recent call last):
  File "/usr/local/lib/python3.6/dist-packages/pyreportjasper/jasperpy.py", line 188, in execute
    self.command, shell=True, check=True).returncode
  File "/usr/lib/python3.6/subprocess.py", line 418, in run
    output=stdout, stderr=stderr)
subprocess.CalledProcessError: Command '/usr/local/lib/python3.6/dist-packages/pyreportjasper/jasperstarter/bin/jasperstarter --locale en_GB process "/main/srhaque/Innovatieltd/testdjango/paiyroll/report/JasperReports/pre_approval.jrxml" -o "/tmp/Acme Accountants/Advanced Mechanics/m1/2018-07-28/pre_approval" -f pdf -t jsonql --data-file '/tmp/Acme Accountants/Advanced Mechanics/m1/2018-07-28/snapshot.json' --jsonql-query 'employee.*'' returned non-zero exit status 1.
[2018-07-02 09:14:55,062: ERROR/ForkPoolWorker-15] Unhandled exception
Traceback (most recent call last):
  File "/usr/local/lib/python3.6/dist-packages/pyreportjasper/jasperpy.py", line 188, in execute
    self.command, shell=True, check=True).returncode
  File "/usr/lib/python3.6/subprocess.py", line 418, in run
    output=stdout, stderr=stderr)
subprocess.CalledProcessError: Command '/usr/local/lib/python3.6/dist-packages/pyreportjasper/jasperstarter/bin/jasperstarter --locale en_GB process "/main/srhaque/Innovatieltd/testdjango/paiyroll/report/JasperReports/pre_approval.jrxml" -o "/tmp/Acme Accountants/Advanced Mechanics/m1/2018-07-28/pre_approval" -f pdf -t jsonql --data-file '/tmp/Acme Accountants/Advanced Mechanics/m1/2018-07-28/snapshot.json' --jsonql-query 'employee.*'' returned non-zero exit status 1.

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  ...
  File "/usr/local/lib/python3.6/dist-packages/pyreportjasper/jasperpy.py", line 151, in process
    return self.execute()
  File "/usr/local/lib/python3.6/dist-packages/pyreportjasper/jasperpy.py", line 193, in execute
    raise NameError('Your report has an error and couldn '
NameError: Your report has an error and couldn 't be processed!\ Try to output the command using the attribute `command;` and run it manually in the console!
=========

(Notice how the word "Command" appears 3 times). With the fix the output is the less redundant:

========
[2018-07-02 09:06:44,706: ERROR/ForkPoolWorker-2] None
Traceback (most recent call last):
  File "/usr/local/lib/python3.6/dist-packages/pyreportjasper/jasperpy.py", line 188, in execute
    self.command, shell=True, check=True).returncode
  File "/usr/lib/python3.6/subprocess.py", line 418, in run
    output=stdout, stderr=stderr)
subprocess.CalledProcessError: Command '/usr/local/lib/python3.6/dist-packages/pyreportjasper/jasperstarter/bin/jasperstarter --locale en_GB process "/main/srhaque/Innovatieltd/testdjango/paiyroll/report/JasperReports/pre_approval.jrxml" -o "/tmp/Acme Accountants/Advanced Mechanics/m1/2018-07-28/pre_approval" -f pdf -t jsonql --data-file '/tmp/Acme Accountants/Advanced Mechanics/m1/2018-07-28/snapshot.json' --jsonql-query 'employee.*'' returned non-zero exit status 1.
[2018-07-02 09:06:44,708: ERROR/ForkPoolWorker-2] Unhandled exception
Traceback (most recent call last):
  File "/usr/local/lib/python3.6/dist-packages/pyreportjasper/jasperpy.py", line 188, in execute
    self.command, shell=True, check=True).returncode
  File "/usr/lib/python3.6/subprocess.py", line 418, in run
    output=stdout, stderr=stderr)
subprocess.CalledProcessError: Command '/usr/local/lib/python3.6/dist-packages/pyreportjasper/jasperstarter/bin/jasperstarter --locale en_GB process "/main/srhaque/Innovatieltd/testdjango/paiyroll/report/JasperReports/pre_approval.jrxml" -o "/tmp/Acme Accountants/Advanced Mechanics/m1/2018-07-28/pre_approval" -f pdf -t jsonql --data-file '/tmp/Acme Accountants/Advanced Mechanics/m1/2018-07-28/snapshot.json' --jsonql-query 'employee.*'' returned non-zero exit status 1.

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  ...
  File "/usr/local/lib/python3.6/dist-packages/pyreportjasper/jasperpy.py", line 151, in process
    return self.execute()
  File "/usr/local/lib/python3.6/dist-packages/pyreportjasper/jasperpy.py", line 193, in execute
    raise NameError('Your report has an error and couldn '
NameError: Your report has an error and couldn 't be processed!\ Try to output the command using the attribute `command;` and run it manually in the console!
========

(Notice how the first "Command" has been replaced by "None", and would have been
replaced by any output from the command for both Python2/3).
@coveralls
Copy link

coveralls commented Jul 2, 2018

Pull Request Test Coverage Report for Build 275

  • 0 of 2 (0.0%) changed or added relevant lines in 1 file are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage increased (+0.5%) to 45.938%

Changes Missing Coverage Covered Lines Changed/Added Lines %
pyreportjasper/jasperpy.py 0 2 0.0%
Files with Coverage Reduction New Missed Lines %
pyreportjasper/jasperpy.py 1 77.48%
Totals Coverage Status
Change from base Build 268: 0.5%
Covered Lines: 4331
Relevant Lines: 9428

💛 - Coveralls

@ShaheedHaque
Copy link
Author

I should mention that you might prefer the line:

    logger.exception(str(e.output))

to be something like:

    logger.exception('Command output: ' + str(e.output))

Let me know if so, and I'll happily oblige.

@ShaheedHaque
Copy link
Author

ShaheedHaque commented Jul 2, 2018

I realised I did something stupid. Sorry. You are actually trying to return the returncode, and completely ignoring the command output. I will rework and do a new PR.

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.

None yet

2 participants