Michael Brown has posted comments on this change. Change subject: IMPALA-4467: Add support for DML statements in stress test ......................................................................
Patch Set 10: (2 comments) http://gerrit.cloudera.org:8080/#/c/5093/9/tests/stress/concurrent_select.py File tests/stress/concurrent_select.py: PS9, Line 1871: if args.reset_databases_before_bin > We would also have to pass in other variables into this function, such as i The code already has methods that do that, like load_random_queries_and_populate_runtime_info(). I still feel this main() method is entirely too long, and adding a long nested function into the middle of it makes readability even worse. Nested functions, closures, etc. are better suited as small declarations that are easily readable. This is difficult to follow. Alex, you've been reviewing this code as well. Do you have any opinion? http://gerrit.cloudera.org:8080/#/c/5093/10/tests/stress/concurrent_select.py File tests/stress/concurrent_select.py: PS10, Line 1687: other(non stress) nit: needs a space -- To view, visit http://gerrit.cloudera.org:8080/5093 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia2aafdc6851cc0e1677a3c668d3350e47c4bfe40 Gerrit-PatchSet: 10 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Taras Bobrovytsky <[email protected]> Gerrit-Reviewer: Alex Behm <[email protected]> Gerrit-Reviewer: Matthew Jacobs <[email protected]> Gerrit-Reviewer: Michael Brown <[email protected]> Gerrit-Reviewer: Taras Bobrovytsky <[email protected]> Gerrit-HasComments: Yes
