David Knupp has posted comments on this change. ( http://gerrit.cloudera.org:8080/9626 )
Change subject: IMPALA-6488: removes use-after-free bug in lib_cache ...................................................................... Patch Set 2: (5 comments) http://gerrit.cloudera.org:8080/#/c/9626/2/tests/query_test/test_udfs.py File tests/query_test/test_udfs.py: http://gerrit.cloudera.org:8080/#/c/9626/2/tests/query_test/test_udfs.py@328 PS2, Line 328: setup = create_fn_to_use.format(unique_database, '/test-warehouse/libTestUdfs.so') Nit: this could probably go before the try block. http://gerrit.cloudera.org:8080/#/c/9626/2/tests/query_test/test_udfs.py@331 PS2, Line 331: print e You can print/log any extra info you like here, but why not then raise the actual exception, rather then forcing an entirely new one using an assert? E.g.: try: setup_client.execute(setup_fn) except Exception as e: print "Unable execute initial create function: {0}".format(setup_fn) # or something... raise http://gerrit.cloudera.org:8080/#/c/9626/2/tests/query_test/test_udfs.py@341 PS2, Line 341: except Exception as e: errors.append(e) PEP-8 discourages inlining like this. Break after the colon (throughout). http://gerrit.cloudera.org:8080/#/c/9626/2/tests/query_test/test_udfs.py@351 PS2, Line 351: except Exception as e: errors.append(e) Break errors.append(e) onto a new line. http://gerrit.cloudera.org:8080/#/c/9626/2/tests/query_test/test_udfs.py@358 PS2, Line 358: for i in range(0, num_uses): Nit: a bit pedantic, but xrange is favored over range in python 2.x. (In fact, range has been simply replaced by xrange in 3.x.). -- To view, visit http://gerrit.cloudera.org:8080/9626 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1f178a2114cb3969dbb920949ddff4e8220b742e Gerrit-Change-Number: 9626 Gerrit-PatchSet: 2 Gerrit-Owner: Vuk Ercegovac <[email protected]> Gerrit-Reviewer: Dan Hecht <[email protected]> Gerrit-Reviewer: David Knupp <[email protected]> Gerrit-Comment-Date: Wed, 14 Mar 2018 21:12:55 +0000 Gerrit-HasComments: Yes
