You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sentry.apache.org by Nachiket Vaidya <nv...@cloudera.com> on 2017/01/26 22:48:17 UTC
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/
-----------------------------------------------------------
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
>
>
Re: Review Request 56000: SENTRY-1604 Sentry JSON message factory:
Need more information in alter partition event
Posted by Hao Hao <ha...@cloudera.com>.
-----------------------------------------------------------
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 Alexander Kolbasov <ak...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56000/#review164613
-----------------------------------------------------------
Ship it!
Ship It!
- Alexander Kolbasov
On Feb. 4, 2017, 5:48 a.m., Nachiket Vaidya wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56000/
> -----------------------------------------------------------
>
> (Updated Feb. 4, 2017, 5:48 a.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
>
>
> 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 Vamsee Yarlagadda <va...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56000/#review164413
-----------------------------------------------------------
Ship it!
Ship It!
- Vamsee Yarlagadda
On Feb. 4, 2017, 5:48 a.m., Nachiket Vaidya wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56000/
> -----------------------------------------------------------
>
> (Updated Feb. 4, 2017, 5:48 a.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
>
>
> 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>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56000/
-----------------------------------------------------------
(Updated Feb. 4, 2017, 5:48 a.m.)
Review request for sentry, Alexander Kolbasov and Hao Hao.
Changes
-------
o + Hao's comments.
Bugs: SENTRY-1604
https://issues.apache.org/jira/browse/SENTRY-1604
Repository: sentry
Description (updated)
-------
SENTRY-1604 Sentry JSON message factory: Need more information in alter partition event
Diffs (updated)
-----
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