> On June 7, 2017, 10:16 p.m., Alexander Kolbasov wrote: > > sentry-hdfs/sentry-hdfs-common/src/main/resources/sentry_hdfs_service.thrift > > Lines 68 (patched) > > <https://reviews.apache.org/r/59895/diff/1/?file=1744052#file1744052line68> > > > > You are changing on the wire ordering of fields - it may be better to > > preserve binary compatibility
Done. I did this change. However, for curiosity, how is thrift compatible when a new column is added at the end? The TPathsUpdate() thrift class still requires the new parameter in order to be constructed. Or is the wire bytes ordered in the same order I define my schema? > On June 7, 2017, 10:16 p.m., Alexander Kolbasov wrote: > > sentry-hdfs/sentry-hdfs-common/src/main/resources/sentry_hdfs_service.thrift > > Line 112 (original), 113 (patched) > > <https://reviews.apache.org/r/59895/diff/1/?file=1744052#file1744052line113> > > > > I would consider adding a new interface rather then modify an old one > > in an incompatible way. Done. I did this change. I had to use a different name because Thrift does not allow method overloading. Instead of get_all_authz_updates_from(), I'm using get_authz_updates(). My concern now is if we do schema changes in the future on this method, then we'll have to use a different method name for compatibility + get_all_authz_updates_from() must be compatible with the new use of HMS image ID, do we want to do this as well? - Sergio ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59895/#review177234 ----------------------------------------------------------- On June 8, 2017, 5:42 p.m., Sergio Pena wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/59895/ > ----------------------------------------------------------- > > (Updated June 8, 2017, 5:42 p.m.) > > > Review request for sentry, Alexander Kolbasov, kalyan kumar kalvagadda, and > Vamsee Yarlagadda. > > > Bugs: SENTRY-1782 > https://issues.apache.org/jira/browse/SENTRY-1782 > > > Repository: sentry > > > Description > ------- > > This adds the HMS image ID to the hdfs/sentry thrift definition. The value > will be used as part of the hdfs/sentry updates requests in order to detect > if a full paths image should be returned or not. > > > Diffs > ----- > > > sentry-hdfs/sentry-hdfs-common/src/gen/thrift/gen-javabean/org/apache/sentry/hdfs/service/thrift/SentryHDFSService.java > 29178d90b744b4b82b92ea24b1dc4f06dbebe36a > > sentry-hdfs/sentry-hdfs-common/src/gen/thrift/gen-javabean/org/apache/sentry/hdfs/service/thrift/TPathsUpdate.java > f3014be9fec01bb32fa171d27bed9ac23bac4717 > > sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/PathsUpdate.java > 6b31f7a4bbff66f6640813774ef79958990d7a57 > > sentry-hdfs/sentry-hdfs-common/src/main/resources/sentry_hdfs_service.thrift > d01841b3bc4a8b38f44e08b45449e11352f6c725 > > sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientDefaultImpl.java > 798bbef98b29af7c885c5ea747d77d2dad6c1693 > > sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceProcessor.java > 395618aa7271b1b739a08f98d284f12c37d31a7c > > > Diff: https://reviews.apache.org/r/59895/diff/2/ > > > Testing > ------- > > > Thanks, > > Sergio Pena > >
