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:35:12 UTC
Review Request 55999: SENTRY-1602 Code cleanup for Sentry JSON message
factory for hive notifications
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55999/
-----------------------------------------------------------
Review request for sentry.
Repository: sentry
Description
-------
SENTRY-1602 Code cleanup for Sentry JSON message factory for hive notifications
Diffs
-----
sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/JSONAlterPartitionMessage.java 890186ba7eecd4aace280d9a1b56a8f01b625b77
sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/JSONAlterTableMessage.java 76211c35f4f5ed158b5c3052f7288cd0083f0d52
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/SentryJSONAlterTableMessage.java 6e59e2568c8fb3526cd7a8ce29b08f5d8f5b0d62
sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONDropPartitionMessage.java 2ab61f7ad5d30a1edb454c3cb239532c736b52e6
sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONMessageDeserializer.java b645c4504ce6e768b31e6f7a1a39b60f73e1ac64
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/TestSentryListenerSentryDeserializer.java 6f1886f942ca80ab4c356b81777d3ac336017292
Diff: https://reviews.apache.org/r/55999/diff/
Testing
-------
Ran unit test cases and they passed.
Thanks,
Nachiket Vaidya
Re: Review Request 55999: SENTRY-1602 Code cleanup for Sentry JSON
message factory for hive notifications
Posted by Nachiket Vaidya <nv...@cloudera.com>.
> On Jan. 30, 2017, 11:24 p.m., Alexander Kolbasov wrote:
> > sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONMessageFactory.java, line 86
> > <https://reviews.apache.org/r/55999/diff/1/?file=1616984#file1616984line86>
> >
> > This can be converted to foreach loop
partitions is an iterator. This is typical use of iterator.
> On Jan. 30, 2017, 11:24 p.m., Alexander Kolbasov wrote:
> > sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONMessageFactory.java, line 156
> > <https://reviews.apache.org/r/55999/diff/1/?file=1616984#file1616984line156>
> >
> > This can be converted to foreach loop
partitions is an iterator. This is typical use of iterator.
> On Jan. 30, 2017, 11:24 p.m., Alexander Kolbasov wrote:
> > sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONMessageDeserializer.java, line 43
> > <https://reviews.apache.org/r/55999/diff/1/?file=1616983#file1616983line43>
> >
> > It isn't that we can't construct it, we can not deserialize it.
> >
> > Same goes for methods below.
This is just replica of JSONMessageDeserializer:
https://github.com/apache/hive/blob/master/hcatalog/server-extensions/src/main/java/org/apache/hive/hcatalog/messaging/json/JSONMessageDeserializer.java
> On Jan. 30, 2017, 11:24 p.m., Alexander Kolbasov wrote:
> > sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONMessageDeserializer.java, line 42
> > <https://reviews.apache.org/r/55999/diff/1/?file=1616983#file1616983line42>
> >
> > readValue throws some specific exceptions - can we be more specific here as well?
This is just replica of JSONMessageDeserializer:
https://github.com/apache/hive/blob/master/hcatalog/server-extensions/src/main/java/org/apache/hive/hcatalog/messaging/json/JSONMessageDeserializer.java
> On Jan. 30, 2017, 11:24 p.m., Alexander Kolbasov wrote:
> > sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONMessageDeserializer.java, line 32
> > <https://reviews.apache.org/r/55999/diff/1/?file=1616983#file1616983line32>
> >
> > IMO this can be removed
The default constructor is required for Jackson.
> On Jan. 30, 2017, 11:24 p.m., Alexander Kolbasov wrote:
> > sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONDropPartitionMessage.java, line 29
> > <https://reviews.apache.org/r/55999/diff/1/?file=1616982#file1616982line29>
> >
> > Can this be private and final?
locations cannot be private final as the default - SentryJSONDropPartitionMessage() it required for jackson.
- Nachiket
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55999/#review163576
-----------------------------------------------------------
On Jan. 26, 2017, 10:49 p.m., Nachiket Vaidya wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55999/
> -----------------------------------------------------------
>
> (Updated Jan. 26, 2017, 10:49 p.m.)
>
>
> Review request for sentry, Alexander Kolbasov and Hao Hao.
>
>
> Bugs: SENTRY-1602
> https://issues.apache.org/jira/browse/SENTRY-1602
>
>
> Repository: sentry
>
>
> Description
> -------
>
> SENTRY-1602 Code cleanup for Sentry JSON message factory for hive notifications
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/JSONAlterPartitionMessage.java 890186ba7eecd4aace280d9a1b56a8f01b625b77
> sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/JSONAlterTableMessage.java 76211c35f4f5ed158b5c3052f7288cd0083f0d52
> 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/SentryJSONAlterTableMessage.java 6e59e2568c8fb3526cd7a8ce29b08f5d8f5b0d62
> sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONDropPartitionMessage.java 2ab61f7ad5d30a1edb454c3cb239532c736b52e6
> sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONMessageDeserializer.java b645c4504ce6e768b31e6f7a1a39b60f73e1ac64
> 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/TestSentryListenerSentryDeserializer.java 6f1886f942ca80ab4c356b81777d3ac336017292
>
> Diff: https://reviews.apache.org/r/55999/diff/
>
>
> Testing
> -------
>
> Ran unit test cases and they passed.
>
>
> Thanks,
>
> Nachiket Vaidya
>
>
Re: Review Request 55999: SENTRY-1602 Code cleanup for Sentry JSON
message factory for hive notifications
Posted by Alexander Kolbasov <ak...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55999/#review163576
-----------------------------------------------------------
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/55999/#comment235040>
can this be private final?
sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONAlterPartitionMessage.java (line 30)
<https://reviews.apache.org/r/55999/#comment235041>
can this be private final?
sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONAlterPartitionMessage.java (line 33)
<https://reviews.apache.org/r/55999/#comment235043>
These are pretty long lines - can you vreak them?
sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONAlterTableMessage.java (line 26)
<https://reviews.apache.org/r/55999/#comment235030>
Can this be private and final?
sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONAlterTableMessage.java (line 28)
<https://reviews.apache.org/r/55999/#comment235031>
Can this be private and final?
sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONAlterTableMessage.java (line 30)
<https://reviews.apache.org/r/55999/#comment235044>
Please break these long lines
sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONDropPartitionMessage.java (line 29)
<https://reviews.apache.org/r/55999/#comment235034>
Can this be private and final?
sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONDropPartitionMessage.java (line 35)
<https://reviews.apache.org/r/55999/#comment235045>
Please break long lines
sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONMessageDeserializer.java (line 26)
<https://reviews.apache.org/r/55999/#comment235052>
can it be private?
sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONMessageDeserializer.java (line 32)
<https://reviews.apache.org/r/55999/#comment235053>
IMO this can be removed
sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONMessageDeserializer.java (line 42)
<https://reviews.apache.org/r/55999/#comment235054>
readValue throws some specific exceptions - can we be more specific here as well?
sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONMessageDeserializer.java (line 43)
<https://reviews.apache.org/r/55999/#comment235055>
It isn't that we can't construct it, we can not deserialize it.
Same goes for methods below.
sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONMessageFactory.java (line 84)
<https://reviews.apache.org/r/55999/#comment235056>
This can be converted to foreach loop
sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONMessageFactory.java (line 134)
<https://reviews.apache.org/r/55999/#comment235057>
This can be converted to foreach loop
- Alexander Kolbasov
On Jan. 26, 2017, 10:49 p.m., Nachiket Vaidya wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55999/
> -----------------------------------------------------------
>
> (Updated Jan. 26, 2017, 10:49 p.m.)
>
>
> Review request for sentry, Alexander Kolbasov and Hao Hao.
>
>
> Bugs: SENTRY-1602
> https://issues.apache.org/jira/browse/SENTRY-1602
>
>
> Repository: sentry
>
>
> Description
> -------
>
> SENTRY-1602 Code cleanup for Sentry JSON message factory for hive notifications
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/JSONAlterPartitionMessage.java 890186ba7eecd4aace280d9a1b56a8f01b625b77
> sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/JSONAlterTableMessage.java 76211c35f4f5ed158b5c3052f7288cd0083f0d52
> 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/SentryJSONAlterTableMessage.java 6e59e2568c8fb3526cd7a8ce29b08f5d8f5b0d62
> sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONDropPartitionMessage.java 2ab61f7ad5d30a1edb454c3cb239532c736b52e6
> sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONMessageDeserializer.java b645c4504ce6e768b31e6f7a1a39b60f73e1ac64
> 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/TestSentryListenerSentryDeserializer.java 6f1886f942ca80ab4c356b81777d3ac336017292
>
> Diff: https://reviews.apache.org/r/55999/diff/
>
>
> Testing
> -------
>
> Ran unit test cases and they passed.
>
>
> Thanks,
>
> Nachiket Vaidya
>
>
Re: Review Request 55999: SENTRY-1602 Code cleanup for Sentry JSON
message factory for hive notifications
Posted by Vamsee Yarlagadda <va...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55999/#review164411
-----------------------------------------------------------
Ship it!
Ship It!
- Vamsee Yarlagadda
On Feb. 4, 2017, 5:42 a.m., Nachiket Vaidya wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55999/
> -----------------------------------------------------------
>
> (Updated Feb. 4, 2017, 5:42 a.m.)
>
>
> Review request for sentry, Alexander Kolbasov and Hao Hao.
>
>
> Bugs: SENTRY-1602
> https://issues.apache.org/jira/browse/SENTRY-1602
>
>
> Repository: sentry
>
>
> Description
> -------
>
> SENTRY-1602 Code cleanup for Sentry JSON message factory for hive notifications
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/JSONAlterPartitionMessage.java 890186ba7eecd4aace280d9a1b56a8f01b625b77
> sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/JSONAlterTableMessage.java 76211c35f4f5ed158b5c3052f7288cd0083f0d52
> sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONAddPartitionMessage.java c0c469c1b1bfcd09494bb611a4a07911f5ef3192
> 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/SentryJSONAlterTableMessage.java 6e59e2568c8fb3526cd7a8ce29b08f5d8f5b0d62
> sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONCreateDatabaseMessage.java ba19cbe42a242ea74f3ea9bf424e799e95d17b3f
> sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONCreateTableMessage.java 57d11d22f2f7c099c8dd354a207dbb297e39be3f
> sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONDropDatabaseMessage.java 05f83f7effee07204e7b0597ac6faf1e0e2e6cdf
> sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONDropPartitionMessage.java 2ab61f7ad5d30a1edb454c3cb239532c736b52e6
> sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONDropTableMessage.java 7005776425a65ab5fdb4b959391c3558000406f4
> sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONMessageDeserializer.java b645c4504ce6e768b31e6f7a1a39b60f73e1ac64
> sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONMessageFactory.java 00e7db8ea4ccf92dae58869fcb21e6e2fcb27103
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java f3cefd6a232bfb91db28f04bebcc98ab3c1ca658
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/TestSentryListenerSentryDeserializer.java 6f1886f942ca80ab4c356b81777d3ac336017292
>
> Diff: https://reviews.apache.org/r/55999/diff/
>
>
> Testing
> -------
>
> Ran unit test cases and they passed.
>
>
> Thanks,
>
> Nachiket Vaidya
>
>
Re: Review Request 55999: SENTRY-1602 Code cleanup for Sentry JSON
message factory for hive notifications
Posted by Nachiket Vaidya <nv...@cloudera.com>.
> On Feb. 6, 2017, 11:40 p.m., Hao Hao wrote:
> > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/AbstractMetastoreTestWithStaticConfiguration.java, line 234
> > <https://reviews.apache.org/r/55999/diff/3/?file=1620737#file1620737line234>
> >
> > Extra line.
Latest revision does not have this change.
- Nachiket
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55999/#review164429
-----------------------------------------------------------
On Feb. 4, 2017, 5:42 a.m., Nachiket Vaidya wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55999/
> -----------------------------------------------------------
>
> (Updated Feb. 4, 2017, 5:42 a.m.)
>
>
> Review request for sentry, Alexander Kolbasov and Hao Hao.
>
>
> Bugs: SENTRY-1602
> https://issues.apache.org/jira/browse/SENTRY-1602
>
>
> Repository: sentry
>
>
> Description
> -------
>
> SENTRY-1602 Code cleanup for Sentry JSON message factory for hive notifications
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/JSONAlterPartitionMessage.java 890186ba7eecd4aace280d9a1b56a8f01b625b77
> sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/JSONAlterTableMessage.java 76211c35f4f5ed158b5c3052f7288cd0083f0d52
> sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONAddPartitionMessage.java c0c469c1b1bfcd09494bb611a4a07911f5ef3192
> 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/SentryJSONAlterTableMessage.java 6e59e2568c8fb3526cd7a8ce29b08f5d8f5b0d62
> sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONCreateDatabaseMessage.java ba19cbe42a242ea74f3ea9bf424e799e95d17b3f
> sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONCreateTableMessage.java 57d11d22f2f7c099c8dd354a207dbb297e39be3f
> sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONDropDatabaseMessage.java 05f83f7effee07204e7b0597ac6faf1e0e2e6cdf
> sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONDropPartitionMessage.java 2ab61f7ad5d30a1edb454c3cb239532c736b52e6
> sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONDropTableMessage.java 7005776425a65ab5fdb4b959391c3558000406f4
> sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONMessageDeserializer.java b645c4504ce6e768b31e6f7a1a39b60f73e1ac64
> sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONMessageFactory.java 00e7db8ea4ccf92dae58869fcb21e6e2fcb27103
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java f3cefd6a232bfb91db28f04bebcc98ab3c1ca658
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/TestSentryListenerSentryDeserializer.java 6f1886f942ca80ab4c356b81777d3ac336017292
>
> Diff: https://reviews.apache.org/r/55999/diff/
>
>
> Testing
> -------
>
> Ran unit test cases and they passed.
>
>
> Thanks,
>
> Nachiket Vaidya
>
>
Re: Review Request 55999: SENTRY-1602 Code cleanup for Sentry JSON
message factory for hive notifications
Posted by Hao Hao <ha...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55999/#review164429
-----------------------------------------------------------
Fix it, then Ship it!
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/AbstractMetastoreTestWithStaticConfiguration.java
<https://reviews.apache.org/r/55999/#comment236143>
Extra line.
- Hao Hao
On Feb. 4, 2017, 5:42 a.m., Nachiket Vaidya wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55999/
> -----------------------------------------------------------
>
> (Updated Feb. 4, 2017, 5:42 a.m.)
>
>
> Review request for sentry, Alexander Kolbasov and Hao Hao.
>
>
> Bugs: SENTRY-1602
> https://issues.apache.org/jira/browse/SENTRY-1602
>
>
> Repository: sentry
>
>
> Description
> -------
>
> SENTRY-1602 Code cleanup for Sentry JSON message factory for hive notifications
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/JSONAlterPartitionMessage.java 890186ba7eecd4aace280d9a1b56a8f01b625b77
> sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/JSONAlterTableMessage.java 76211c35f4f5ed158b5c3052f7288cd0083f0d52
> sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONAddPartitionMessage.java c0c469c1b1bfcd09494bb611a4a07911f5ef3192
> 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/SentryJSONAlterTableMessage.java 6e59e2568c8fb3526cd7a8ce29b08f5d8f5b0d62
> sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONCreateDatabaseMessage.java ba19cbe42a242ea74f3ea9bf424e799e95d17b3f
> sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONCreateTableMessage.java 57d11d22f2f7c099c8dd354a207dbb297e39be3f
> sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONDropDatabaseMessage.java 05f83f7effee07204e7b0597ac6faf1e0e2e6cdf
> sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONDropPartitionMessage.java 2ab61f7ad5d30a1edb454c3cb239532c736b52e6
> sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONDropTableMessage.java 7005776425a65ab5fdb4b959391c3558000406f4
> sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONMessageDeserializer.java b645c4504ce6e768b31e6f7a1a39b60f73e1ac64
> sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONMessageFactory.java 00e7db8ea4ccf92dae58869fcb21e6e2fcb27103
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java f3cefd6a232bfb91db28f04bebcc98ab3c1ca658
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/TestSentryListenerSentryDeserializer.java 6f1886f942ca80ab4c356b81777d3ac336017292
>
> Diff: https://reviews.apache.org/r/55999/diff/
>
>
> Testing
> -------
>
> Ran unit test cases and they passed.
>
>
> Thanks,
>
> Nachiket Vaidya
>
>
Re: Review Request 55999: SENTRY-1602 Code cleanup for Sentry JSON
message factory for hive notifications
Posted by Nachiket Vaidya <nv...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55999/
-----------------------------------------------------------
(Updated Feb. 4, 2017, 5:42 a.m.)
Review request for sentry, Alexander Kolbasov and Hao Hao.
Changes
-------
o + Shasha's + Hao's comments.
Bugs: SENTRY-1602
https://issues.apache.org/jira/browse/SENTRY-1602
Repository: sentry
Description
-------
SENTRY-1602 Code cleanup for Sentry JSON message factory for hive notifications
Diffs (updated)
-----
sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/JSONAlterPartitionMessage.java 890186ba7eecd4aace280d9a1b56a8f01b625b77
sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/JSONAlterTableMessage.java 76211c35f4f5ed158b5c3052f7288cd0083f0d52
sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONAddPartitionMessage.java c0c469c1b1bfcd09494bb611a4a07911f5ef3192
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/SentryJSONAlterTableMessage.java 6e59e2568c8fb3526cd7a8ce29b08f5d8f5b0d62
sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONCreateDatabaseMessage.java ba19cbe42a242ea74f3ea9bf424e799e95d17b3f
sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONCreateTableMessage.java 57d11d22f2f7c099c8dd354a207dbb297e39be3f
sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONDropDatabaseMessage.java 05f83f7effee07204e7b0597ac6faf1e0e2e6cdf
sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONDropPartitionMessage.java 2ab61f7ad5d30a1edb454c3cb239532c736b52e6
sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONDropTableMessage.java 7005776425a65ab5fdb4b959391c3558000406f4
sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONMessageDeserializer.java b645c4504ce6e768b31e6f7a1a39b60f73e1ac64
sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONMessageFactory.java 00e7db8ea4ccf92dae58869fcb21e6e2fcb27103
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java f3cefd6a232bfb91db28f04bebcc98ab3c1ca658
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/TestSentryListenerSentryDeserializer.java 6f1886f942ca80ab4c356b81777d3ac336017292
Diff: https://reviews.apache.org/r/55999/diff/
Testing
-------
Ran unit test cases and they passed.
Thanks,
Nachiket Vaidya
Re: Review Request 55999: SENTRY-1602 Code cleanup for Sentry JSON
message factory for hive notifications
Posted by Hao Hao <ha...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55999/#review164218
-----------------------------------------------------------
sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONAlterTableMessage.java (line 26)
<https://reviews.apache.org/r/55999/#comment235873>
yeah if you do not mind. Thanks!
- Hao Hao
On Jan. 31, 2017, 11:27 p.m., Nachiket Vaidya wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55999/
> -----------------------------------------------------------
>
> (Updated Jan. 31, 2017, 11:27 p.m.)
>
>
> Review request for sentry, Alexander Kolbasov and Hao Hao.
>
>
> Bugs: SENTRY-1602
> https://issues.apache.org/jira/browse/SENTRY-1602
>
>
> Repository: sentry
>
>
> Description
> -------
>
> SENTRY-1602 Code cleanup for Sentry JSON message factory for hive notifications
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/JSONAlterPartitionMessage.java 890186ba7eecd4aace280d9a1b56a8f01b625b77
> sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/JSONAlterTableMessage.java 76211c35f4f5ed158b5c3052f7288cd0083f0d52
> 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/SentryJSONAlterTableMessage.java 6e59e2568c8fb3526cd7a8ce29b08f5d8f5b0d62
> sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONDropPartitionMessage.java 2ab61f7ad5d30a1edb454c3cb239532c736b52e6
> sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONMessageDeserializer.java b645c4504ce6e768b31e6f7a1a39b60f73e1ac64
> 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/TestSentryListenerSentryDeserializer.java 6f1886f942ca80ab4c356b81777d3ac336017292
>
> Diff: https://reviews.apache.org/r/55999/diff/
>
>
> Testing
> -------
>
> Ran unit test cases and they passed.
>
>
> Thanks,
>
> Nachiket Vaidya
>
>
Re: Review Request 55999: SENTRY-1602 Code cleanup for Sentry JSON
message factory for hive notifications
Posted by Hao Hao <ha...@cloudera.com>.
> On Feb. 3, 2017, 7:51 p.m., Hao Hao wrote:
> > sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONAlterTableMessage.java, line 26
> > <https://reviews.apache.org/r/55999/diff/3/?file=1620733#file1620733line26>
> >
> > Can you name it to newLocation?
>
> Nachiket Vaidya wrote:
> This is existing code. Do you still want to change it?
What do you mean by existing code? It is used in somewhere? I feel it would be better to have an accurate name here.
- Hao
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55999/#review164147
-----------------------------------------------------------
On Jan. 31, 2017, 11:27 p.m., Nachiket Vaidya wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55999/
> -----------------------------------------------------------
>
> (Updated Jan. 31, 2017, 11:27 p.m.)
>
>
> Review request for sentry, Alexander Kolbasov and Hao Hao.
>
>
> Bugs: SENTRY-1602
> https://issues.apache.org/jira/browse/SENTRY-1602
>
>
> Repository: sentry
>
>
> Description
> -------
>
> SENTRY-1602 Code cleanup for Sentry JSON message factory for hive notifications
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/JSONAlterPartitionMessage.java 890186ba7eecd4aace280d9a1b56a8f01b625b77
> sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/JSONAlterTableMessage.java 76211c35f4f5ed158b5c3052f7288cd0083f0d52
> 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/SentryJSONAlterTableMessage.java 6e59e2568c8fb3526cd7a8ce29b08f5d8f5b0d62
> sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONDropPartitionMessage.java 2ab61f7ad5d30a1edb454c3cb239532c736b52e6
> sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONMessageDeserializer.java b645c4504ce6e768b31e6f7a1a39b60f73e1ac64
> 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/TestSentryListenerSentryDeserializer.java 6f1886f942ca80ab4c356b81777d3ac336017292
>
> Diff: https://reviews.apache.org/r/55999/diff/
>
>
> Testing
> -------
>
> Ran unit test cases and they passed.
>
>
> Thanks,
>
> Nachiket Vaidya
>
>
Re: Review Request 55999: SENTRY-1602 Code cleanup for Sentry JSON
message factory for hive notifications
Posted by Nachiket Vaidya <nv...@cloudera.com>.
> On Feb. 3, 2017, 7:51 p.m., Hao Hao wrote:
> > sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONAlterTableMessage.java, line 26
> > <https://reviews.apache.org/r/55999/diff/3/?file=1620733#file1620733line26>
> >
> > Can you name it to newLocation?
>
> Nachiket Vaidya wrote:
> This is existing code. Do you still want to change it?
>
> Hao Hao wrote:
> What do you mean by existing code? It is used in somewhere? I feel it would be better to have an accurate name here.
I meant choosing the name 'location' was done before my change. Do we want to have different name?
- Nachiket
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55999/#review164147
-----------------------------------------------------------
On Jan. 31, 2017, 11:27 p.m., Nachiket Vaidya wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55999/
> -----------------------------------------------------------
>
> (Updated Jan. 31, 2017, 11:27 p.m.)
>
>
> Review request for sentry, Alexander Kolbasov and Hao Hao.
>
>
> Bugs: SENTRY-1602
> https://issues.apache.org/jira/browse/SENTRY-1602
>
>
> Repository: sentry
>
>
> Description
> -------
>
> SENTRY-1602 Code cleanup for Sentry JSON message factory for hive notifications
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/JSONAlterPartitionMessage.java 890186ba7eecd4aace280d9a1b56a8f01b625b77
> sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/JSONAlterTableMessage.java 76211c35f4f5ed158b5c3052f7288cd0083f0d52
> 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/SentryJSONAlterTableMessage.java 6e59e2568c8fb3526cd7a8ce29b08f5d8f5b0d62
> sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONDropPartitionMessage.java 2ab61f7ad5d30a1edb454c3cb239532c736b52e6
> sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONMessageDeserializer.java b645c4504ce6e768b31e6f7a1a39b60f73e1ac64
> 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/TestSentryListenerSentryDeserializer.java 6f1886f942ca80ab4c356b81777d3ac336017292
>
> Diff: https://reviews.apache.org/r/55999/diff/
>
>
> Testing
> -------
>
> Ran unit test cases and they passed.
>
>
> Thanks,
>
> Nachiket Vaidya
>
>
Re: Review Request 55999: SENTRY-1602 Code cleanup for Sentry JSON
message factory for hive notifications
Posted by Nachiket Vaidya <nv...@cloudera.com>.
> On Feb. 3, 2017, 7:51 p.m., Hao Hao wrote:
> > sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONAlterTableMessage.java, line 26
> > <https://reviews.apache.org/r/55999/diff/3/?file=1620733#file1620733line26>
> >
> > Can you name it to newLocation?
This is existing code. Do you still want to change it?
- Nachiket
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55999/#review164147
-----------------------------------------------------------
On Jan. 31, 2017, 11:27 p.m., Nachiket Vaidya wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55999/
> -----------------------------------------------------------
>
> (Updated Jan. 31, 2017, 11:27 p.m.)
>
>
> Review request for sentry, Alexander Kolbasov and Hao Hao.
>
>
> Bugs: SENTRY-1602
> https://issues.apache.org/jira/browse/SENTRY-1602
>
>
> Repository: sentry
>
>
> Description
> -------
>
> SENTRY-1602 Code cleanup for Sentry JSON message factory for hive notifications
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/JSONAlterPartitionMessage.java 890186ba7eecd4aace280d9a1b56a8f01b625b77
> sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/JSONAlterTableMessage.java 76211c35f4f5ed158b5c3052f7288cd0083f0d52
> 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/SentryJSONAlterTableMessage.java 6e59e2568c8fb3526cd7a8ce29b08f5d8f5b0d62
> sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONDropPartitionMessage.java 2ab61f7ad5d30a1edb454c3cb239532c736b52e6
> sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONMessageDeserializer.java b645c4504ce6e768b31e6f7a1a39b60f73e1ac64
> 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/TestSentryListenerSentryDeserializer.java 6f1886f942ca80ab4c356b81777d3ac336017292
>
> Diff: https://reviews.apache.org/r/55999/diff/
>
>
> Testing
> -------
>
> Ran unit test cases and they passed.
>
>
> Thanks,
>
> Nachiket Vaidya
>
>
Re: Review Request 55999: SENTRY-1602 Code cleanup for Sentry JSON
message factory for hive notifications
Posted by Hao Hao <ha...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55999/#review164147
-----------------------------------------------------------
Fix it, then Ship it!
sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONAlterTableMessage.java (line 26)
<https://reviews.apache.org/r/55999/#comment235734>
Can you name it to newLocation?
sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONDropPartitionMessage.java (line 29)
<https://reviews.apache.org/r/55999/#comment235737>
private?
- Hao Hao
On Jan. 31, 2017, 11:27 p.m., Nachiket Vaidya wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55999/
> -----------------------------------------------------------
>
> (Updated Jan. 31, 2017, 11:27 p.m.)
>
>
> Review request for sentry, Alexander Kolbasov and Hao Hao.
>
>
> Bugs: SENTRY-1602
> https://issues.apache.org/jira/browse/SENTRY-1602
>
>
> Repository: sentry
>
>
> Description
> -------
>
> SENTRY-1602 Code cleanup for Sentry JSON message factory for hive notifications
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/JSONAlterPartitionMessage.java 890186ba7eecd4aace280d9a1b56a8f01b625b77
> sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/JSONAlterTableMessage.java 76211c35f4f5ed158b5c3052f7288cd0083f0d52
> 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/SentryJSONAlterTableMessage.java 6e59e2568c8fb3526cd7a8ce29b08f5d8f5b0d62
> sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONDropPartitionMessage.java 2ab61f7ad5d30a1edb454c3cb239532c736b52e6
> sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONMessageDeserializer.java b645c4504ce6e768b31e6f7a1a39b60f73e1ac64
> 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/TestSentryListenerSentryDeserializer.java 6f1886f942ca80ab4c356b81777d3ac336017292
>
> Diff: https://reviews.apache.org/r/55999/diff/
>
>
> Testing
> -------
>
> Ran unit test cases and they passed.
>
>
> Thanks,
>
> Nachiket Vaidya
>
>
Re: Review Request 55999: SENTRY-1602 Code cleanup for Sentry JSON
message factory for hive notifications
Posted by Hao Hao <ha...@cloudera.com>.
> On Feb. 3, 2017, 7:32 p.m., Hao Hao wrote:
> > sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONMessageFactory.java, line 60
> > <https://reviews.apache.org/r/55999/diff/3/?file=1620736#file1620736line60>
> >
> > While doing SENTRY-1587, I realize when we dropping Database, we also need the location of all the tables belong to this database. You can refer to my initial patch. Could you please add it here?
>
> Nachiket Vaidya wrote:
> When drop database cascade is called, drop table notifications are created for all tables of the database along with drop database. So we are ok.
>
> Hao Hao wrote:
> From what I discovered, it seems onDropTable of MetaStoreEventListener is not triggered in case of dropping databse. So we may still need it.
Double checked, it looks like onDropTable is actaully being invoked in this case. So closing this comment.
- Hao
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55999/#review164144
-----------------------------------------------------------
On Feb. 4, 2017, 5:42 a.m., Nachiket Vaidya wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55999/
> -----------------------------------------------------------
>
> (Updated Feb. 4, 2017, 5:42 a.m.)
>
>
> Review request for sentry, Alexander Kolbasov and Hao Hao.
>
>
> Bugs: SENTRY-1602
> https://issues.apache.org/jira/browse/SENTRY-1602
>
>
> Repository: sentry
>
>
> Description
> -------
>
> SENTRY-1602 Code cleanup for Sentry JSON message factory for hive notifications
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/JSONAlterPartitionMessage.java 890186ba7eecd4aace280d9a1b56a8f01b625b77
> sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/JSONAlterTableMessage.java 76211c35f4f5ed158b5c3052f7288cd0083f0d52
> sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONAddPartitionMessage.java c0c469c1b1bfcd09494bb611a4a07911f5ef3192
> 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/SentryJSONAlterTableMessage.java 6e59e2568c8fb3526cd7a8ce29b08f5d8f5b0d62
> sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONCreateDatabaseMessage.java ba19cbe42a242ea74f3ea9bf424e799e95d17b3f
> sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONCreateTableMessage.java 57d11d22f2f7c099c8dd354a207dbb297e39be3f
> sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONDropDatabaseMessage.java 05f83f7effee07204e7b0597ac6faf1e0e2e6cdf
> sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONDropPartitionMessage.java 2ab61f7ad5d30a1edb454c3cb239532c736b52e6
> sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONDropTableMessage.java 7005776425a65ab5fdb4b959391c3558000406f4
> sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONMessageDeserializer.java b645c4504ce6e768b31e6f7a1a39b60f73e1ac64
> sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONMessageFactory.java 00e7db8ea4ccf92dae58869fcb21e6e2fcb27103
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java f3cefd6a232bfb91db28f04bebcc98ab3c1ca658
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/TestSentryListenerSentryDeserializer.java 6f1886f942ca80ab4c356b81777d3ac336017292
>
> Diff: https://reviews.apache.org/r/55999/diff/
>
>
> Testing
> -------
>
> Ran unit test cases and they passed.
>
>
> Thanks,
>
> Nachiket Vaidya
>
>
Re: Review Request 55999: SENTRY-1602 Code cleanup for Sentry JSON
message factory for hive notifications
Posted by Nachiket Vaidya <nv...@cloudera.com>.
> On Feb. 3, 2017, 7:32 p.m., Hao Hao wrote:
> > sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONMessageFactory.java, line 60
> > <https://reviews.apache.org/r/55999/diff/3/?file=1620736#file1620736line60>
> >
> > While doing SENTRY-1587, I realize when we dropping Database, we also need the location of all the tables belong to this database. You can refer to my initial patch. Could you please add it here?
When drop database cascade is called, drop table notifications are created for all tables of the database along with drop database. So we are ok.
- Nachiket
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55999/#review164144
-----------------------------------------------------------
On Jan. 31, 2017, 11:27 p.m., Nachiket Vaidya wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55999/
> -----------------------------------------------------------
>
> (Updated Jan. 31, 2017, 11:27 p.m.)
>
>
> Review request for sentry, Alexander Kolbasov and Hao Hao.
>
>
> Bugs: SENTRY-1602
> https://issues.apache.org/jira/browse/SENTRY-1602
>
>
> Repository: sentry
>
>
> Description
> -------
>
> SENTRY-1602 Code cleanup for Sentry JSON message factory for hive notifications
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/JSONAlterPartitionMessage.java 890186ba7eecd4aace280d9a1b56a8f01b625b77
> sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/JSONAlterTableMessage.java 76211c35f4f5ed158b5c3052f7288cd0083f0d52
> 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/SentryJSONAlterTableMessage.java 6e59e2568c8fb3526cd7a8ce29b08f5d8f5b0d62
> sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONDropPartitionMessage.java 2ab61f7ad5d30a1edb454c3cb239532c736b52e6
> sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONMessageDeserializer.java b645c4504ce6e768b31e6f7a1a39b60f73e1ac64
> 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/TestSentryListenerSentryDeserializer.java 6f1886f942ca80ab4c356b81777d3ac336017292
>
> Diff: https://reviews.apache.org/r/55999/diff/
>
>
> Testing
> -------
>
> Ran unit test cases and they passed.
>
>
> Thanks,
>
> Nachiket Vaidya
>
>
Re: Review Request 55999: SENTRY-1602 Code cleanup for Sentry JSON
message factory for hive notifications
Posted by Hao Hao <ha...@cloudera.com>.
> On Feb. 3, 2017, 7:32 p.m., Hao Hao wrote:
> > sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONMessageFactory.java, line 60
> > <https://reviews.apache.org/r/55999/diff/3/?file=1620736#file1620736line60>
> >
> > While doing SENTRY-1587, I realize when we dropping Database, we also need the location of all the tables belong to this database. You can refer to my initial patch. Could you please add it here?
>
> Nachiket Vaidya wrote:
> When drop database cascade is called, drop table notifications are created for all tables of the database along with drop database. So we are ok.
From what I discovered, it seems onDropTable of MetaStoreEventListener is not triggered in case of dropping databse. So we may still need it.
- Hao
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55999/#review164144
-----------------------------------------------------------
On Jan. 31, 2017, 11:27 p.m., Nachiket Vaidya wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55999/
> -----------------------------------------------------------
>
> (Updated Jan. 31, 2017, 11:27 p.m.)
>
>
> Review request for sentry, Alexander Kolbasov and Hao Hao.
>
>
> Bugs: SENTRY-1602
> https://issues.apache.org/jira/browse/SENTRY-1602
>
>
> Repository: sentry
>
>
> Description
> -------
>
> SENTRY-1602 Code cleanup for Sentry JSON message factory for hive notifications
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/JSONAlterPartitionMessage.java 890186ba7eecd4aace280d9a1b56a8f01b625b77
> sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/JSONAlterTableMessage.java 76211c35f4f5ed158b5c3052f7288cd0083f0d52
> 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/SentryJSONAlterTableMessage.java 6e59e2568c8fb3526cd7a8ce29b08f5d8f5b0d62
> sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONDropPartitionMessage.java 2ab61f7ad5d30a1edb454c3cb239532c736b52e6
> sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONMessageDeserializer.java b645c4504ce6e768b31e6f7a1a39b60f73e1ac64
> 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/TestSentryListenerSentryDeserializer.java 6f1886f942ca80ab4c356b81777d3ac336017292
>
> Diff: https://reviews.apache.org/r/55999/diff/
>
>
> Testing
> -------
>
> Ran unit test cases and they passed.
>
>
> Thanks,
>
> Nachiket Vaidya
>
>
Re: Review Request 55999: SENTRY-1602 Code cleanup for Sentry JSON
message factory for hive notifications
Posted by Hao Hao <ha...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55999/#review164144
-----------------------------------------------------------
sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONMessageFactory.java (line 58)
<https://reviews.apache.org/r/55999/#comment235732>
While doing SENTRY-1587, I realize when we dropping Database, we also need the location of all the tables belong to this database. You can refer to my initial patch. Could you please add it here?
- Hao Hao
On Jan. 31, 2017, 11:27 p.m., Nachiket Vaidya wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55999/
> -----------------------------------------------------------
>
> (Updated Jan. 31, 2017, 11:27 p.m.)
>
>
> Review request for sentry, Alexander Kolbasov and Hao Hao.
>
>
> Bugs: SENTRY-1602
> https://issues.apache.org/jira/browse/SENTRY-1602
>
>
> Repository: sentry
>
>
> Description
> -------
>
> SENTRY-1602 Code cleanup for Sentry JSON message factory for hive notifications
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/JSONAlterPartitionMessage.java 890186ba7eecd4aace280d9a1b56a8f01b625b77
> sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/JSONAlterTableMessage.java 76211c35f4f5ed158b5c3052f7288cd0083f0d52
> 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/SentryJSONAlterTableMessage.java 6e59e2568c8fb3526cd7a8ce29b08f5d8f5b0d62
> sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONDropPartitionMessage.java 2ab61f7ad5d30a1edb454c3cb239532c736b52e6
> sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONMessageDeserializer.java b645c4504ce6e768b31e6f7a1a39b60f73e1ac64
> 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/TestSentryListenerSentryDeserializer.java 6f1886f942ca80ab4c356b81777d3ac336017292
>
> Diff: https://reviews.apache.org/r/55999/diff/
>
>
> Testing
> -------
>
> Ran unit test cases and they passed.
>
>
> Thanks,
>
> Nachiket Vaidya
>
>
Re: Review Request 55999: SENTRY-1602 Code cleanup for Sentry JSON
message factory for hive notifications
Posted by Alexander Kolbasov <ak...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55999/#review164082
-----------------------------------------------------------
Fix it, then Ship it!
Ship It!
sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONAlterPartitionMessage.java (line 29)
<https://reviews.apache.org/r/55999/#comment235863>
Can location and oldLocation be private as well?
Same goes for other similar classses
sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONMessageFactory.java (line 82)
<https://reviews.apache.org/r/55999/#comment235642>
IMO using foreach is cleaner then using iterators.
This comment applies here and below in multiple places.
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/AbstractMetastoreTestWithStaticConfiguration.java
<https://reviews.apache.org/r/55999/#comment235640>
This looks like a stray change
- Alexander Kolbasov
On Jan. 31, 2017, 11:27 p.m., Nachiket Vaidya wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55999/
> -----------------------------------------------------------
>
> (Updated Jan. 31, 2017, 11:27 p.m.)
>
>
> Review request for sentry, Alexander Kolbasov and Hao Hao.
>
>
> Bugs: SENTRY-1602
> https://issues.apache.org/jira/browse/SENTRY-1602
>
>
> Repository: sentry
>
>
> Description
> -------
>
> SENTRY-1602 Code cleanup for Sentry JSON message factory for hive notifications
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/JSONAlterPartitionMessage.java 890186ba7eecd4aace280d9a1b56a8f01b625b77
> sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/JSONAlterTableMessage.java 76211c35f4f5ed158b5c3052f7288cd0083f0d52
> 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/SentryJSONAlterTableMessage.java 6e59e2568c8fb3526cd7a8ce29b08f5d8f5b0d62
> sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONDropPartitionMessage.java 2ab61f7ad5d30a1edb454c3cb239532c736b52e6
> sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONMessageDeserializer.java b645c4504ce6e768b31e6f7a1a39b60f73e1ac64
> 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/TestSentryListenerSentryDeserializer.java 6f1886f942ca80ab4c356b81777d3ac336017292
>
> Diff: https://reviews.apache.org/r/55999/diff/
>
>
> Testing
> -------
>
> Ran unit test cases and they passed.
>
>
> Thanks,
>
> Nachiket Vaidya
>
>
Re: Review Request 55999: SENTRY-1602 Code cleanup for Sentry JSON
message factory for hive notifications
Posted by Nachiket Vaidya <nv...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55999/
-----------------------------------------------------------
(Updated Jan. 31, 2017, 11:27 p.m.)
Review request for sentry, Alexander Kolbasov and Hao Hao.
Changes
-------
o unit test case fixes.
Bugs: SENTRY-1602
https://issues.apache.org/jira/browse/SENTRY-1602
Repository: sentry
Description
-------
SENTRY-1602 Code cleanup for Sentry JSON message factory for hive notifications
Diffs (updated)
-----
sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/JSONAlterPartitionMessage.java 890186ba7eecd4aace280d9a1b56a8f01b625b77
sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/JSONAlterTableMessage.java 76211c35f4f5ed158b5c3052f7288cd0083f0d52
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/SentryJSONAlterTableMessage.java 6e59e2568c8fb3526cd7a8ce29b08f5d8f5b0d62
sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONDropPartitionMessage.java 2ab61f7ad5d30a1edb454c3cb239532c736b52e6
sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONMessageDeserializer.java b645c4504ce6e768b31e6f7a1a39b60f73e1ac64
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/TestSentryListenerSentryDeserializer.java 6f1886f942ca80ab4c356b81777d3ac336017292
Diff: https://reviews.apache.org/r/55999/diff/
Testing
-------
Ran unit test cases and they passed.
Thanks,
Nachiket Vaidya
Re: Review Request 55999: SENTRY-1602 Code cleanup for Sentry JSON
message factory for hive notifications
Posted by Nachiket Vaidya <nv...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55999/
-----------------------------------------------------------
(Updated Jan. 31, 2017, 6:18 a.m.)
Review request for sentry, Alexander Kolbasov and Hao Hao.
Changes
-------
o + Shasha's comments.
Bugs: SENTRY-1602
https://issues.apache.org/jira/browse/SENTRY-1602
Repository: sentry
Description
-------
SENTRY-1602 Code cleanup for Sentry JSON message factory for hive notifications
Diffs (updated)
-----
sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/JSONAlterPartitionMessage.java 890186ba7eecd4aace280d9a1b56a8f01b625b77
sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/JSONAlterTableMessage.java 76211c35f4f5ed158b5c3052f7288cd0083f0d52
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/SentryJSONAlterTableMessage.java 6e59e2568c8fb3526cd7a8ce29b08f5d8f5b0d62
sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONDropPartitionMessage.java 2ab61f7ad5d30a1edb454c3cb239532c736b52e6
sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONMessageDeserializer.java b645c4504ce6e768b31e6f7a1a39b60f73e1ac64
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/TestSentryListenerSentryDeserializer.java 6f1886f942ca80ab4c356b81777d3ac336017292
Diff: https://reviews.apache.org/r/55999/diff/
Testing
-------
Ran unit test cases and they passed.
Thanks,
Nachiket Vaidya
Re: Review Request 55999: SENTRY-1602 Code cleanup for Sentry JSON
message factory for hive notifications
Posted by Nachiket Vaidya <nv...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55999/
-----------------------------------------------------------
(Updated Jan. 26, 2017, 10:49 p.m.)
Review request for sentry, Alexander Kolbasov and Hao Hao.
Bugs: SENTRY-1602
https://issues.apache.org/jira/browse/SENTRY-1602
Repository: sentry
Description
-------
SENTRY-1602 Code cleanup for Sentry JSON message factory for hive notifications
Diffs
-----
sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/JSONAlterPartitionMessage.java 890186ba7eecd4aace280d9a1b56a8f01b625b77
sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/JSONAlterTableMessage.java 76211c35f4f5ed158b5c3052f7288cd0083f0d52
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/SentryJSONAlterTableMessage.java 6e59e2568c8fb3526cd7a8ce29b08f5d8f5b0d62
sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONDropPartitionMessage.java 2ab61f7ad5d30a1edb454c3cb239532c736b52e6
sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONMessageDeserializer.java b645c4504ce6e768b31e6f7a1a39b60f73e1ac64
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/TestSentryListenerSentryDeserializer.java 6f1886f942ca80ab4c356b81777d3ac336017292
Diff: https://reviews.apache.org/r/55999/diff/
Testing
-------
Ran unit test cases and they passed.
Thanks,
Nachiket Vaidya