You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sentry.apache.org by Hao Hao <ha...@cloudera.com> on 2017/02/03 20:34:00 UTC
Re: Review Request 56000: SENTRY-1604 Sentry JSON message factory:
Need more information in alter partition event
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56000/#review164160
-----------------------------------------------------------
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/#comment235748>
Can you rename it to newLocation?
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/#comment235745>
newPartitions? Also private?
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/TestDBNotificationListenerInBuiltDeserializer.java (line 351)
<https://reviews.apache.org/r/56000/#comment235747>
Is this comment still valid?
- Hao Hao
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
>
>
Re: Review Request 56000: SENTRY-1604 Sentry JSON message factory:
Need more information in alter partition event
Posted by Nachiket Vaidya <nv...@cloudera.com>.
> 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
>
>