Comment 7 for bug 1336462

Revision history for this message
Martin Pitt (pitti) wrote :

Comment 4, proposed hook:

> if arch in [ "ppc64", "ppc64le"]:
> print "Processing power %s" %arch

Please use print(...) to be compatible with Python 3. Ubuntu has used Apport with Python3 for several releases already. Also, please use 4 spaces for indentation, but I can fix such small issues myself when integrating this into the package, so nevermind that.

> else:
> print "Not executing power query since arch %s" %arch

print()

> exit(1)
Please move that entire check to add_info(), and replace the exit(1) with "return".

> def add_copy_spec(path):
> for file in apport.hookutils.read_file(path):

That won't work with directory names as "path" arguments, they are not files. If it's a single hierarchy, you can use "for file in os.listdir(path):", for a fully recursive search you need to use os.walk(). But please don't attach a gazillion little files; for e. g. /device-tree/ I suppose you should rather call tar and attach the tarball only. Or you pre-process it locally to extract a minimum of the actually interesting information and then only attach that text file.

Same print → print() issue as before.

Please dont' call root_command_output() multiple times, you'll get an auth prompt for every single one. Do a single call to attach_root_command_outputs() instead with a mapping of the commands.

Some hints:

You can get help for a Python class, or method with:

$ python3
>>> import apport.hookutils
>>> help(apport.hookutils)

>>> help(apport.hookutils.attach_root_command_outputs)

You can test your hook by putting it into /usr/share/apport/general-hooks/, then run "ubuntu-bug coreutils" from the CLI and inspect the details. You'll probably see a lot of exceptions (due to the Python 2 print statement, etc) first. Once you are satisfied wkth the kind of output you get in the hook, I'm happy to take over and massage the code a little more for readability/PEP-8/etc.

Thanks!