mirror of
https://github.com/apache/impala.git
synced 2025-12-25 02:03:09 -05:00
db0f0dadf1
Adds gerrit comments for changes in Thrift/FlatBuffers files that could break the communication between impalad and catalogd/statestore during upgrade. Basically, only new optional fields can be added in Thrift protocol. For Flatbuffers schemas, we should only add new fields at the end of a table definition. Adds a new option (--revision) for critique-gerrit-review.py to specify the revision (HEAD or a commit, branch, etc). Also adds an option (--base-revision) to specify the base revision for comparison. To test the script locally, prepare a virtual env with the virtualenv package installed: virtualenv venv source venv/bin/activate pip install virtualenv Then run the script with --dryrun: python bin/jenkins/critique-gerrit-review.py --dryrun --revisioneffc9df93Limitations - False positive in cases that add new Thrift structs with required fields and only use those new structs in new optional fields, e.g.effc9df93and72732da9d. - Might have false positive results on reformat changes due to simple string checks, e.g.91d8a8f62. - Can't check incompatible changes in FlatBuffers files. Just add general file level comments. We can integrate DUPCheck in the future to parse the Thrift/FlatBuffers files to AST and compare the AST instead. https://github.com/jwjwyoung/DUPChecker Tests: - Verified incompatible commits like012996a06and65094a74f. - Verified posting Gerrit comments from local env using my username. Change-Id: Ib35fafa50bfd38631312d22464df14d426f55346 Reviewed-on: http://gerrit.cloudera.org:8080/21646 Reviewed-by: Riza Suminto <riza.suminto@cloudera.com> Tested-by: Quanlong Huang <huangquanlong@gmail.com>