> On Feb. 3, 2017, 8:34 p.m., Hao Hao wrote: > > sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONAlterPartitionMessage.java, > > line 32 > > <https://reviews.apache.org/r/56000/diff/1/?file=1617005#file1617005line32> > > > > newPartitions? Also private?
There is already field "values". This is its counterpart. > On Feb. 3, 2017, 8:34 p.m., Hao Hao wrote: > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/TestDBNotificationListenerInBuiltDeserializer.java, > > line 351 > > <https://reviews.apache.org/r/56000/diff/1/?file=1617008#file1617008line351> > > > > Is this comment still valid? agree. can be removed. > On Feb. 3, 2017, 8:34 p.m., Hao Hao wrote: > > sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONAlterPartitionMessage.java, > > line 28 > > <https://reviews.apache.org/r/56000/diff/1/?file=1617005#file1617005line28> > > > > Can you rename it to newLocation? This is existing code. Do you still want to change it? If yes, we need to change in alter table message as well. - Nachiket ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56000/#review164160 ----------------------------------------------------------- On Jan. 26, 2017, 10:48 p.m., Nachiket Vaidya wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/56000/ > ----------------------------------------------------------- > > (Updated Jan. 26, 2017, 10:48 p.m.) > > > Review request for sentry, Alexander Kolbasov and Hao Hao. > > > Bugs: SENTRY-1604 > https://issues.apache.org/jira/browse/SENTRY-1604 > > > Repository: sentry > > > Description > ------- > > SENTRY-1604 Sentry JSON message factory: Need more information in alter > partition event > > Currently, message factory for alter partition does not store new partition > values. It just stores old values. For alter partition with partition change, > we need both values. > In this fix, sentry message factory stores also new partition values in alter > partition message in addition to old values. Added unit test cases for the > same. > > > Diffs > ----- > > > sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONAlterPartitionMessage.java > 99eb67a61363616af663a9be579b2e3a3344fd69 > > sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONMessageFactory.java > 00e7db8ea4ccf92dae58869fcb21e6e2fcb27103 > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/AbstractMetastoreTestWithStaticConfiguration.java > 567b4c86339a9494647dccc980df2b427225907f > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/TestDBNotificationListenerInBuiltDeserializer.java > 56e19c4d371918975fda127bf6f2fd32d98ed328 > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/TestSentryListenerSentryDeserializer.java > 6f1886f942ca80ab4c356b81777d3ac336017292 > > Diff: https://reviews.apache.org/r/56000/diff/ > > > Testing > ------- > > Added unit test cases. > > > Thanks, > > Nachiket Vaidya > >
