Hello Riza Suminto, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/21646
to look at the new patch set (#8).
Change subject: IMPALA-13240: Add gerrit comments for Thrift/FlatBuffers changes
......................................................................
IMPALA-13240: Add gerrit comments for Thrift/FlatBuffers changes
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 --revision effc9df93
Limitations
- False positive in cases that add new Thrift structs with required
fields and only use those new structs in new optional fields, e.g.
effc9df93 and 72732da9d.
- 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 like 012996a06 and 65094a74f.
- Verified posting Gerrit comments from local env using my username.
Change-Id: Ib35fafa50bfd38631312d22464df14d426f55346
---
M bin/jenkins/critique-gerrit-review.py
M common/thrift/Data.thrift
2 files changed, 211 insertions(+), 26 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/46/21646/8
--
To view, visit http://gerrit.cloudera.org:8080/21646
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib35fafa50bfd38631312d22464df14d426f55346
Gerrit-Change-Number: 21646
Gerrit-PatchSet: 8
Gerrit-Owner: Quanlong Huang <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Quanlong Huang <[email protected]>
Gerrit-Reviewer: Riza Suminto <[email protected]>
