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 2016/12/14 02:35:58 UTC

Review Request 54729: SENTRY-1538: Create schema for storing HMS path change and Sentry permission change.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54729/
-----------------------------------------------------------

Review request for sentry, Alexander Kolbasov, kalyan kumar kalvagadda, and Vamsee Yarlagadda.


Repository: sentry


Description
-------

Create schema for storing HMS path change and Sentry permission change. It contains change ID, timestamp and the change event in JSON format.


Diffs
-----

  sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/PathsUpdate.java 7cfb3bf57bd1a425b07df6c08db31b9691dd17f5 
  sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/PermissionsUpdate.java 98349232bc658c39791e58b64949ecb975fff7a0 
  sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/Updateable.java 4dc3a0cebdff89ee2f9070e4d822a28dbd164c08 
  sentry-hdfs/sentry-hdfs-common/src/test/java/org/apache/sentry/hdfs/TestPathsUpdate.java 53243b44329997e64ab70db5e5d8c3614ab974d6 
  sentry-hdfs/sentry-hdfs-common/src/test/java/org/apache/sentry/hdfs/TestPermissionUpdate.java PRE-CREATION 
  sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/UpdateForwarder.java ea1c8f62f300abd03206fdd9ec76fbf216fc6a2d 
  sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/TestUpdateForwarder.java 315d4b3a80b9ab3ac8704ecbab81876f141423f1 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPathChange.java PRE-CREATION 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPermChange.java PRE-CREATION 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/package.jdo fb5470f635e03c762beb5bc2c6b06641b2476ce9 

Diff: https://reviews.apache.org/r/54729/diff/


Testing
-------


Thanks,

Hao Hao


Re: Review Request 54729: SENTRY-1538: Create schema for storing HMS path change and Sentry permission change.

Posted by Hao Hao <ha...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54729/#review161019
-----------------------------------------------------------




sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPathChange.java (line 82)
<https://reviews.apache.org/r/54729/#comment232284>

    Using 31 is to use an odd prime to generate the hashcode. More reference here: http://stackoverflow.com/questions/299304/why-does-javas-hashcode-in-string-use-31-as-a-multiplier


- Hao Hao


On Dec. 14, 2016, 2:35 a.m., Hao Hao wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54729/
> -----------------------------------------------------------
> 
> (Updated Dec. 14, 2016, 2:35 a.m.)
> 
> 
> Review request for sentry, Alexander Kolbasov, kalyan kumar kalvagadda, and Vamsee Yarlagadda.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> Create schema for storing HMS path change and Sentry permission change. It contains change ID, timestamp and the change event in JSON format.
> 
> 
> Diffs
> -----
> 
>   sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/PathsUpdate.java 7cfb3bf57bd1a425b07df6c08db31b9691dd17f5 
>   sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/PermissionsUpdate.java 98349232bc658c39791e58b64949ecb975fff7a0 
>   sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/Updateable.java 4dc3a0cebdff89ee2f9070e4d822a28dbd164c08 
>   sentry-hdfs/sentry-hdfs-common/src/test/java/org/apache/sentry/hdfs/TestPathsUpdate.java 53243b44329997e64ab70db5e5d8c3614ab974d6 
>   sentry-hdfs/sentry-hdfs-common/src/test/java/org/apache/sentry/hdfs/TestPermissionUpdate.java PRE-CREATION 
>   sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/UpdateForwarder.java ea1c8f62f300abd03206fdd9ec76fbf216fc6a2d 
>   sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/TestUpdateForwarder.java 315d4b3a80b9ab3ac8704ecbab81876f141423f1 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPathChange.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPermChange.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/package.jdo fb5470f635e03c762beb5bc2c6b06641b2476ce9 
> 
> Diff: https://reviews.apache.org/r/54729/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Hao Hao
> 
>


Re: Review Request 54729: SENTRY-1538: Create schema for storing HMS path change and Sentry permission change.

Posted by kalyan kumar kalvagadda <kk...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54729/#review160730
-----------------------------------------------------------




sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPermChange.java (line 49)
<https://reviews.apache.org/r/54729/#comment231928>

    Will this function be used?


- kalyan kumar kalvagadda


On Dec. 14, 2016, 2:35 a.m., Hao Hao wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54729/
> -----------------------------------------------------------
> 
> (Updated Dec. 14, 2016, 2:35 a.m.)
> 
> 
> Review request for sentry, Alexander Kolbasov, kalyan kumar kalvagadda, and Vamsee Yarlagadda.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> Create schema for storing HMS path change and Sentry permission change. It contains change ID, timestamp and the change event in JSON format.
> 
> 
> Diffs
> -----
> 
>   sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/PathsUpdate.java 7cfb3bf57bd1a425b07df6c08db31b9691dd17f5 
>   sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/PermissionsUpdate.java 98349232bc658c39791e58b64949ecb975fff7a0 
>   sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/Updateable.java 4dc3a0cebdff89ee2f9070e4d822a28dbd164c08 
>   sentry-hdfs/sentry-hdfs-common/src/test/java/org/apache/sentry/hdfs/TestPathsUpdate.java 53243b44329997e64ab70db5e5d8c3614ab974d6 
>   sentry-hdfs/sentry-hdfs-common/src/test/java/org/apache/sentry/hdfs/TestPermissionUpdate.java PRE-CREATION 
>   sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/UpdateForwarder.java ea1c8f62f300abd03206fdd9ec76fbf216fc6a2d 
>   sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/TestUpdateForwarder.java 315d4b3a80b9ab3ac8704ecbab81876f141423f1 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPathChange.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPermChange.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/package.jdo fb5470f635e03c762beb5bc2c6b06641b2476ce9 
> 
> Diff: https://reviews.apache.org/r/54729/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Hao Hao
> 
>


Re: Review Request 54729: SENTRY-1538: Create schema for storing HMS path change and Sentry permission change.

Posted by Hao Hao <ha...@cloudera.com>.

> On Jan. 10, 2017, 8:24 p.m., Alexander Kolbasov wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPermChange.java, line 87
> > <https://reviews.apache.org/r/54729/diff/1-2/?file=1583712#file1583712line87>
> >
> >     iWhat is the point of multiplying 31 by one? Can you add some comment explaining the hash function?

Do you have a better recommendation for the hash function implementation? It looks like it is a common practice to use 31 muliplication for hash code generation to obtain a unique value?  http://stackoverflow.com/questions/299304/why-does-javas-hashcode-in-string-use-31-as-a-multiplier, http://stackoverflow.com/questions/3613102/why-use-a-prime-number-in-hashcode


- Hao


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54729/#review161110
-----------------------------------------------------------


On Jan. 10, 2017, 1:57 a.m., Hao Hao wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54729/
> -----------------------------------------------------------
> 
> (Updated Jan. 10, 2017, 1:57 a.m.)
> 
> 
> Review request for sentry, Alexander Kolbasov, kalyan kumar kalvagadda, and Vamsee Yarlagadda.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> Create schema for storing HMS path change and Sentry permission change. It contains change ID, timestamp and the change event in JSON format.
> 
> 
> Diffs
> -----
> 
>   sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/PathsUpdate.java 7cfb3bf57bd1a425b07df6c08db31b9691dd17f5 
>   sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/PermissionsUpdate.java 98349232bc658c39791e58b64949ecb975fff7a0 
>   sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/ThriftSerializer.java b66f70ba3f9cec6b44dc853ed09494be37303e2f 
>   sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/Updateable.java 4dc3a0cebdff89ee2f9070e4d822a28dbd164c08 
>   sentry-hdfs/sentry-hdfs-common/src/test/java/org/apache/sentry/hdfs/TestPathsUpdate.java 53243b44329997e64ab70db5e5d8c3614ab974d6 
>   sentry-hdfs/sentry-hdfs-common/src/test/java/org/apache/sentry/hdfs/TestPermissionUpdate.java PRE-CREATION 
>   sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/UpdateForwarder.java ea1c8f62f300abd03206fdd9ec76fbf216fc6a2d 
>   sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/TestUpdateForwarder.java 315d4b3a80b9ab3ac8704ecbab81876f141423f1 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPathChange.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPermChange.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/package.jdo fb5470f635e03c762beb5bc2c6b06641b2476ce9 
> 
> Diff: https://reviews.apache.org/r/54729/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Hao Hao
> 
>


Re: Review Request 54729: SENTRY-1538: Create schema for storing HMS path change and Sentry permission change.

Posted by Alexander Kolbasov <ak...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54729/#review161110
-----------------------------------------------------------


Fix it, then Ship it!





sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPermChange.java (line 82)
<https://reviews.apache.org/r/54729/#comment232348>

    iWhat is the point of multiplying 31 by one? Can you add some comment explaining the hash function?



sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/ThriftSerializer.java (line 64)
<https://reviews.apache.org/r/54729/#comment232349>

    Can we pre-create both serializers as static final fields? This way we don't need to create them for every serde op. If we do that, we can just expose toString/fromString methodfs here.



sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/Updateable.java (line 45)
<https://reviews.apache.org/r/54729/#comment232350>

    Please change this to proper Javadoc and add description for parameters and return values.



sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPathChange.java (line 26)
<https://reviews.apache.org/r/54729/#comment232351>

    Nit: need a space between &lt and Hive, otherwise it doesn't show as '<'



sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPermChange.java (line 28)
<https://reviews.apache.org/r/54729/#comment232353>

    It would be nice to show some example of permission change JSON object.



sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPermChange.java (line 29)
<https://reviews.apache.org/r/54729/#comment232352>

    Nit: add <p>


- Alexander Kolbasov


On Jan. 10, 2017, 1:57 a.m., Hao Hao wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54729/
> -----------------------------------------------------------
> 
> (Updated Jan. 10, 2017, 1:57 a.m.)
> 
> 
> Review request for sentry, Alexander Kolbasov, kalyan kumar kalvagadda, and Vamsee Yarlagadda.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> Create schema for storing HMS path change and Sentry permission change. It contains change ID, timestamp and the change event in JSON format.
> 
> 
> Diffs
> -----
> 
>   sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/PathsUpdate.java 7cfb3bf57bd1a425b07df6c08db31b9691dd17f5 
>   sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/PermissionsUpdate.java 98349232bc658c39791e58b64949ecb975fff7a0 
>   sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/ThriftSerializer.java b66f70ba3f9cec6b44dc853ed09494be37303e2f 
>   sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/Updateable.java 4dc3a0cebdff89ee2f9070e4d822a28dbd164c08 
>   sentry-hdfs/sentry-hdfs-common/src/test/java/org/apache/sentry/hdfs/TestPathsUpdate.java 53243b44329997e64ab70db5e5d8c3614ab974d6 
>   sentry-hdfs/sentry-hdfs-common/src/test/java/org/apache/sentry/hdfs/TestPermissionUpdate.java PRE-CREATION 
>   sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/UpdateForwarder.java ea1c8f62f300abd03206fdd9ec76fbf216fc6a2d 
>   sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/TestUpdateForwarder.java 315d4b3a80b9ab3ac8704ecbab81876f141423f1 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPathChange.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPermChange.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/package.jdo fb5470f635e03c762beb5bc2c6b06641b2476ce9 
> 
> Diff: https://reviews.apache.org/r/54729/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Hao Hao
> 
>


Re: Review Request 54729: SENTRY-1538: Create schema for storing HMS path change and Sentry permission change.

Posted by Hao Hao <ha...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54729/
-----------------------------------------------------------

(Updated Jan. 11, 2017, 1:13 a.m.)


Review request for sentry, Alexander Kolbasov, kalyan kumar kalvagadda, and Vamsee Yarlagadda.


Repository: sentry


Description
-------

Create schema for storing HMS path change and Sentry permission change. It contains change ID, timestamp and the change event in JSON format.


Diffs (updated)
-----

  sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/PathsUpdate.java 7cfb3bf57bd1a425b07df6c08db31b9691dd17f5 
  sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/PermissionsUpdate.java 98349232bc658c39791e58b64949ecb975fff7a0 
  sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/ThriftSerializer.java b66f70ba3f9cec6b44dc853ed09494be37303e2f 
  sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/Updateable.java 8391b85ddee857d7910afc1e79fd6b8a7d32e400 
  sentry-hdfs/sentry-hdfs-common/src/test/java/org/apache/sentry/hdfs/TestPathsUpdate.java 53243b44329997e64ab70db5e5d8c3614ab974d6 
  sentry-hdfs/sentry-hdfs-common/src/test/java/org/apache/sentry/hdfs/TestPermissionUpdate.java PRE-CREATION 
  sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/UpdateForwarder.java 8131350dc3c0430dfb3d92624bdb80eec58dbf4f 
  sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/TestUpdateForwarder.java 315d4b3a80b9ab3ac8704ecbab81876f141423f1 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPathChange.java PRE-CREATION 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPermChange.java PRE-CREATION 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/package.jdo fb5470f635e03c762beb5bc2c6b06641b2476ce9 

Diff: https://reviews.apache.org/r/54729/diff/


Testing
-------


Thanks,

Hao Hao


Re: Review Request 54729: SENTRY-1538: Create schema for storing HMS path change and Sentry permission change.

Posted by Hao Hao <ha...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54729/
-----------------------------------------------------------

(Updated Jan. 10, 2017, 11:49 p.m.)


Review request for sentry, Alexander Kolbasov, kalyan kumar kalvagadda, and Vamsee Yarlagadda.


Repository: sentry


Description
-------

Create schema for storing HMS path change and Sentry permission change. It contains change ID, timestamp and the change event in JSON format.


Diffs (updated)
-----

  sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/PathsUpdate.java 7cfb3bf57bd1a425b07df6c08db31b9691dd17f5 
  sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/PermissionsUpdate.java 98349232bc658c39791e58b64949ecb975fff7a0 
  sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/ThriftSerializer.java b66f70ba3f9cec6b44dc853ed09494be37303e2f 
  sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/Updateable.java 8391b85ddee857d7910afc1e79fd6b8a7d32e400 
  sentry-hdfs/sentry-hdfs-common/src/test/java/org/apache/sentry/hdfs/TestPathsUpdate.java 53243b44329997e64ab70db5e5d8c3614ab974d6 
  sentry-hdfs/sentry-hdfs-common/src/test/java/org/apache/sentry/hdfs/TestPermissionUpdate.java PRE-CREATION 
  sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/UpdateForwarder.java 8131350dc3c0430dfb3d92624bdb80eec58dbf4f 
  sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/TestUpdateForwarder.java 315d4b3a80b9ab3ac8704ecbab81876f141423f1 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPathChange.java PRE-CREATION 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPermChange.java PRE-CREATION 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/package.jdo fb5470f635e03c762beb5bc2c6b06641b2476ce9 

Diff: https://reviews.apache.org/r/54729/diff/


Testing
-------


Thanks,

Hao Hao


Re: Review Request 54729: SENTRY-1538: Create schema for storing HMS path change and Sentry permission change.

Posted by Hao Hao <ha...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54729/
-----------------------------------------------------------

(Updated Jan. 10, 2017, 9:48 p.m.)


Review request for sentry, Alexander Kolbasov, kalyan kumar kalvagadda, and Vamsee Yarlagadda.


Repository: sentry


Description
-------

Create schema for storing HMS path change and Sentry permission change. It contains change ID, timestamp and the change event in JSON format.


Diffs (updated)
-----

  sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/PathsUpdate.java 7cfb3bf57bd1a425b07df6c08db31b9691dd17f5 
  sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/PermissionsUpdate.java 98349232bc658c39791e58b64949ecb975fff7a0 
  sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/ThriftSerializer.java b66f70ba3f9cec6b44dc853ed09494be37303e2f 
  sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/Updateable.java 8391b85ddee857d7910afc1e79fd6b8a7d32e400 
  sentry-hdfs/sentry-hdfs-common/src/test/java/org/apache/sentry/hdfs/TestPathsUpdate.java 53243b44329997e64ab70db5e5d8c3614ab974d6 
  sentry-hdfs/sentry-hdfs-common/src/test/java/org/apache/sentry/hdfs/TestPermissionUpdate.java PRE-CREATION 
  sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/UpdateForwarder.java 8131350dc3c0430dfb3d92624bdb80eec58dbf4f 
  sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/TestUpdateForwarder.java 315d4b3a80b9ab3ac8704ecbab81876f141423f1 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPathChange.java PRE-CREATION 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPermChange.java PRE-CREATION 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/package.jdo fb5470f635e03c762beb5bc2c6b06641b2476ce9 

Diff: https://reviews.apache.org/r/54729/diff/


Testing
-------


Thanks,

Hao Hao


Re: Review Request 54729: SENTRY-1538: Create schema for storing HMS path change and Sentry permission change.

Posted by Hao Hao <ha...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54729/
-----------------------------------------------------------

(Updated Jan. 10, 2017, 1:57 a.m.)


Review request for sentry, Alexander Kolbasov, kalyan kumar kalvagadda, and Vamsee Yarlagadda.


Repository: sentry


Description
-------

Create schema for storing HMS path change and Sentry permission change. It contains change ID, timestamp and the change event in JSON format.


Diffs (updated)
-----

  sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/PathsUpdate.java 7cfb3bf57bd1a425b07df6c08db31b9691dd17f5 
  sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/PermissionsUpdate.java 98349232bc658c39791e58b64949ecb975fff7a0 
  sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/ThriftSerializer.java b66f70ba3f9cec6b44dc853ed09494be37303e2f 
  sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/Updateable.java 4dc3a0cebdff89ee2f9070e4d822a28dbd164c08 
  sentry-hdfs/sentry-hdfs-common/src/test/java/org/apache/sentry/hdfs/TestPathsUpdate.java 53243b44329997e64ab70db5e5d8c3614ab974d6 
  sentry-hdfs/sentry-hdfs-common/src/test/java/org/apache/sentry/hdfs/TestPermissionUpdate.java PRE-CREATION 
  sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/UpdateForwarder.java ea1c8f62f300abd03206fdd9ec76fbf216fc6a2d 
  sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/TestUpdateForwarder.java 315d4b3a80b9ab3ac8704ecbab81876f141423f1 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPathChange.java PRE-CREATION 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPermChange.java PRE-CREATION 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/package.jdo fb5470f635e03c762beb5bc2c6b06641b2476ce9 

Diff: https://reviews.apache.org/r/54729/diff/


Testing
-------


Thanks,

Hao Hao


Re: Review Request 54729: SENTRY-1538: Create schema for storing HMS path change and Sentry permission change.

Posted by Hao Hao <ha...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54729/#review160748
-----------------------------------------------------------




sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPermChange.java (line 49)
<https://reviews.apache.org/r/54729/#comment231945>

    Will fixed. Thanks for catching it!


- Hao Hao


On Dec. 14, 2016, 2:35 a.m., Hao Hao wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54729/
> -----------------------------------------------------------
> 
> (Updated Dec. 14, 2016, 2:35 a.m.)
> 
> 
> Review request for sentry, Alexander Kolbasov, kalyan kumar kalvagadda, and Vamsee Yarlagadda.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> Create schema for storing HMS path change and Sentry permission change. It contains change ID, timestamp and the change event in JSON format.
> 
> 
> Diffs
> -----
> 
>   sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/PathsUpdate.java 7cfb3bf57bd1a425b07df6c08db31b9691dd17f5 
>   sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/PermissionsUpdate.java 98349232bc658c39791e58b64949ecb975fff7a0 
>   sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/Updateable.java 4dc3a0cebdff89ee2f9070e4d822a28dbd164c08 
>   sentry-hdfs/sentry-hdfs-common/src/test/java/org/apache/sentry/hdfs/TestPathsUpdate.java 53243b44329997e64ab70db5e5d8c3614ab974d6 
>   sentry-hdfs/sentry-hdfs-common/src/test/java/org/apache/sentry/hdfs/TestPermissionUpdate.java PRE-CREATION 
>   sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/UpdateForwarder.java ea1c8f62f300abd03206fdd9ec76fbf216fc6a2d 
>   sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/TestUpdateForwarder.java 315d4b3a80b9ab3ac8704ecbab81876f141423f1 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPathChange.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPermChange.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/package.jdo fb5470f635e03c762beb5bc2c6b06641b2476ce9 
> 
> Diff: https://reviews.apache.org/r/54729/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Hao Hao
> 
>


Re: Review Request 54729: SENTRY-1538: Create schema for storing HMS path change and Sentry permission change.

Posted by Hao Hao <ha...@cloudera.com>.

> On Jan. 6, 2017, 10:28 p.m., Vamsee Yarlagadda wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPathChange.java, lines 73-87
> > <https://reviews.apache.org/r/54729/diff/1/?file=1583711#file1583711line73>
> >
> >     Just curious to know how datanucleus leverages these overriden methods like hashCode, toString, equals ?

Datanucleus use hashCode, and equals to determine equality of a persistable object. More reference: http://www.datanucleus.org/products/accessplatform/jdo/orm/one_to_many_collection.html


- Hao


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54729/#review160776
-----------------------------------------------------------


On Dec. 14, 2016, 2:35 a.m., Hao Hao wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54729/
> -----------------------------------------------------------
> 
> (Updated Dec. 14, 2016, 2:35 a.m.)
> 
> 
> Review request for sentry, Alexander Kolbasov, kalyan kumar kalvagadda, and Vamsee Yarlagadda.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> Create schema for storing HMS path change and Sentry permission change. It contains change ID, timestamp and the change event in JSON format.
> 
> 
> Diffs
> -----
> 
>   sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/PathsUpdate.java 7cfb3bf57bd1a425b07df6c08db31b9691dd17f5 
>   sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/PermissionsUpdate.java 98349232bc658c39791e58b64949ecb975fff7a0 
>   sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/Updateable.java 4dc3a0cebdff89ee2f9070e4d822a28dbd164c08 
>   sentry-hdfs/sentry-hdfs-common/src/test/java/org/apache/sentry/hdfs/TestPathsUpdate.java 53243b44329997e64ab70db5e5d8c3614ab974d6 
>   sentry-hdfs/sentry-hdfs-common/src/test/java/org/apache/sentry/hdfs/TestPermissionUpdate.java PRE-CREATION 
>   sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/UpdateForwarder.java ea1c8f62f300abd03206fdd9ec76fbf216fc6a2d 
>   sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/TestUpdateForwarder.java 315d4b3a80b9ab3ac8704ecbab81876f141423f1 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPathChange.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPermChange.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/package.jdo fb5470f635e03c762beb5bc2c6b06641b2476ce9 
> 
> Diff: https://reviews.apache.org/r/54729/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Hao Hao
> 
>


Re: Review Request 54729: SENTRY-1538: Create schema for storing HMS path change and Sentry permission change.

Posted by Vamsee Yarlagadda <va...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54729/#review160776
-----------------------------------------------------------


Fix it, then Ship it!




LGTM otherwise.


sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPathChange.java (lines 73 - 87)
<https://reviews.apache.org/r/54729/#comment231994>

    Just curious to know how datanucleus leverages these overriden methods like hashCode, toString, equals ?


- Vamsee Yarlagadda


On Dec. 14, 2016, 2:35 a.m., Hao Hao wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54729/
> -----------------------------------------------------------
> 
> (Updated Dec. 14, 2016, 2:35 a.m.)
> 
> 
> Review request for sentry, Alexander Kolbasov, kalyan kumar kalvagadda, and Vamsee Yarlagadda.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> Create schema for storing HMS path change and Sentry permission change. It contains change ID, timestamp and the change event in JSON format.
> 
> 
> Diffs
> -----
> 
>   sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/PathsUpdate.java 7cfb3bf57bd1a425b07df6c08db31b9691dd17f5 
>   sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/PermissionsUpdate.java 98349232bc658c39791e58b64949ecb975fff7a0 
>   sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/Updateable.java 4dc3a0cebdff89ee2f9070e4d822a28dbd164c08 
>   sentry-hdfs/sentry-hdfs-common/src/test/java/org/apache/sentry/hdfs/TestPathsUpdate.java 53243b44329997e64ab70db5e5d8c3614ab974d6 
>   sentry-hdfs/sentry-hdfs-common/src/test/java/org/apache/sentry/hdfs/TestPermissionUpdate.java PRE-CREATION 
>   sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/UpdateForwarder.java ea1c8f62f300abd03206fdd9ec76fbf216fc6a2d 
>   sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/TestUpdateForwarder.java 315d4b3a80b9ab3ac8704ecbab81876f141423f1 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPathChange.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPermChange.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/package.jdo fb5470f635e03c762beb5bc2c6b06641b2476ce9 
> 
> Diff: https://reviews.apache.org/r/54729/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Hao Hao
> 
>


Re: Review Request 54729: SENTRY-1538: Create schema for storing HMS path change and Sentry permission change.

Posted by Hao Hao <ha...@cloudera.com>.

> On Jan. 6, 2017, 9:38 p.m., Alexander Kolbasov wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPathChange.java, line 82
> > <https://reviews.apache.org/r/54729/diff/1/?file=1583711#file1583711line82>
> >
> >     What is the idea here of multiplying 31 by one?
> >     And why changeId is not participating in the calculation?
> >     
> >     Can we just use changeId as the hashcode?
> 
> Hao Hao wrote:
>     Will add changeID as the hashcode as well

Using 31 is to use an odd prime to generate the hashcode. More reference here: http://stackoverflow.com/questions/299304/why-does-javas-hashcode-in-string-use-31-as-a-multiplier


- Hao


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54729/#review159412
-----------------------------------------------------------


On Dec. 14, 2016, 2:35 a.m., Hao Hao wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54729/
> -----------------------------------------------------------
> 
> (Updated Dec. 14, 2016, 2:35 a.m.)
> 
> 
> Review request for sentry, Alexander Kolbasov, kalyan kumar kalvagadda, and Vamsee Yarlagadda.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> Create schema for storing HMS path change and Sentry permission change. It contains change ID, timestamp and the change event in JSON format.
> 
> 
> Diffs
> -----
> 
>   sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/PathsUpdate.java 7cfb3bf57bd1a425b07df6c08db31b9691dd17f5 
>   sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/PermissionsUpdate.java 98349232bc658c39791e58b64949ecb975fff7a0 
>   sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/Updateable.java 4dc3a0cebdff89ee2f9070e4d822a28dbd164c08 
>   sentry-hdfs/sentry-hdfs-common/src/test/java/org/apache/sentry/hdfs/TestPathsUpdate.java 53243b44329997e64ab70db5e5d8c3614ab974d6 
>   sentry-hdfs/sentry-hdfs-common/src/test/java/org/apache/sentry/hdfs/TestPermissionUpdate.java PRE-CREATION 
>   sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/UpdateForwarder.java ea1c8f62f300abd03206fdd9ec76fbf216fc6a2d 
>   sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/TestUpdateForwarder.java 315d4b3a80b9ab3ac8704ecbab81876f141423f1 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPathChange.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPermChange.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/package.jdo fb5470f635e03c762beb5bc2c6b06641b2476ce9 
> 
> Diff: https://reviews.apache.org/r/54729/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Hao Hao
> 
>


Re: Review Request 54729: SENTRY-1538: Create schema for storing HMS path change and Sentry permission change.

Posted by Hao Hao <ha...@cloudera.com>.

> On Jan. 6, 2017, 9:38 p.m., Alexander Kolbasov wrote:
> > sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/PathsUpdate.java, line 161
> > <https://reviews.apache.org/r/54729/diff/1/?file=1583704#file1583704line161>
> >
> >     Okease add javadocs for the new functions

Just realized this is a useless method which was added when generating json. Will remove it for now.


> On Jan. 6, 2017, 9:38 p.m., Alexander Kolbasov wrote:
> > sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/PathsUpdate.java, line 162
> > <https://reviews.apache.org/r/54729/diff/1/?file=1583704#file1583704line162>
> >
> >     Can we define "/" as a constant (if it isn't in some hadoop libraries that we use already)?

Same as above. Will remove it.


> On Jan. 6, 2017, 9:38 p.m., Alexander Kolbasov wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPathChange.java, line 82
> > <https://reviews.apache.org/r/54729/diff/1/?file=1583711#file1583711line82>
> >
> >     What is the idea here of multiplying 31 by one?
> >     And why changeId is not participating in the calculation?
> >     
> >     Can we just use changeId as the hashcode?

Will add changeID as the hashcode as well


- Hao


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54729/#review159412
-----------------------------------------------------------


On Dec. 14, 2016, 2:35 a.m., Hao Hao wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54729/
> -----------------------------------------------------------
> 
> (Updated Dec. 14, 2016, 2:35 a.m.)
> 
> 
> Review request for sentry, Alexander Kolbasov, kalyan kumar kalvagadda, and Vamsee Yarlagadda.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> Create schema for storing HMS path change and Sentry permission change. It contains change ID, timestamp and the change event in JSON format.
> 
> 
> Diffs
> -----
> 
>   sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/PathsUpdate.java 7cfb3bf57bd1a425b07df6c08db31b9691dd17f5 
>   sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/PermissionsUpdate.java 98349232bc658c39791e58b64949ecb975fff7a0 
>   sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/Updateable.java 4dc3a0cebdff89ee2f9070e4d822a28dbd164c08 
>   sentry-hdfs/sentry-hdfs-common/src/test/java/org/apache/sentry/hdfs/TestPathsUpdate.java 53243b44329997e64ab70db5e5d8c3614ab974d6 
>   sentry-hdfs/sentry-hdfs-common/src/test/java/org/apache/sentry/hdfs/TestPermissionUpdate.java PRE-CREATION 
>   sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/UpdateForwarder.java ea1c8f62f300abd03206fdd9ec76fbf216fc6a2d 
>   sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/TestUpdateForwarder.java 315d4b3a80b9ab3ac8704ecbab81876f141423f1 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPathChange.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPermChange.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/package.jdo fb5470f635e03c762beb5bc2c6b06641b2476ce9 
> 
> Diff: https://reviews.apache.org/r/54729/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Hao Hao
> 
>


Re: Review Request 54729: SENTRY-1538: Create schema for storing HMS path change and Sentry permission change.

Posted by Alexander Kolbasov <ak...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54729/#review159412
-----------------------------------------------------------




sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/PathsUpdate.java (line 161)
<https://reviews.apache.org/r/54729/#comment231969>

    Okease add javadocs for the new functions



sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/PathsUpdate.java (line 162)
<https://reviews.apache.org/r/54729/#comment231942>

    Can we define "/" as a constant (if it isn't in some hadoop libraries that we use already)?



sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/PermissionsUpdate.java (line 113)
<https://reviews.apache.org/r/54729/#comment231977>

    Can we have a common static functions for serialization/deserialization that are used both here and in PathsUpdate so that we don't duplicate code?



sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/Updateable.java (line 44)
<https://reviews.apache.org/r/54729/#comment231973>

    Please add javadoc for methods



sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/Updateable.java (line 45)
<https://reviews.apache.org/r/54729/#comment231975>

    Naming - may be change to JSONSerialize and JSONDeserialize?



sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPathChange.java (line 26)
<https://reviews.apache.org/r/54729/#comment230427>

    You need to use some HTML formatting here - use &lt and &gt for <> and <p> for empty line - otherwise javadoc looks as one big long line.



sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPathChange.java (line 82)
<https://reviews.apache.org/r/54729/#comment230428>

    What is the idea here of multiplying 31 by one?
    And why changeId is not participating in the calculation?
    
    Can we just use changeId as the hashcode?



sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPermChange.java (line 29)
<https://reviews.apache.org/r/54729/#comment230430>

    Add <p>


- Alexander Kolbasov


On Dec. 14, 2016, 2:35 a.m., Hao Hao wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54729/
> -----------------------------------------------------------
> 
> (Updated Dec. 14, 2016, 2:35 a.m.)
> 
> 
> Review request for sentry, Alexander Kolbasov, kalyan kumar kalvagadda, and Vamsee Yarlagadda.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> Create schema for storing HMS path change and Sentry permission change. It contains change ID, timestamp and the change event in JSON format.
> 
> 
> Diffs
> -----
> 
>   sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/PathsUpdate.java 7cfb3bf57bd1a425b07df6c08db31b9691dd17f5 
>   sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/PermissionsUpdate.java 98349232bc658c39791e58b64949ecb975fff7a0 
>   sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/Updateable.java 4dc3a0cebdff89ee2f9070e4d822a28dbd164c08 
>   sentry-hdfs/sentry-hdfs-common/src/test/java/org/apache/sentry/hdfs/TestPathsUpdate.java 53243b44329997e64ab70db5e5d8c3614ab974d6 
>   sentry-hdfs/sentry-hdfs-common/src/test/java/org/apache/sentry/hdfs/TestPermissionUpdate.java PRE-CREATION 
>   sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/UpdateForwarder.java ea1c8f62f300abd03206fdd9ec76fbf216fc6a2d 
>   sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/TestUpdateForwarder.java 315d4b3a80b9ab3ac8704ecbab81876f141423f1 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPathChange.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPermChange.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/package.jdo fb5470f635e03c762beb5bc2c6b06641b2476ce9 
> 
> Diff: https://reviews.apache.org/r/54729/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Hao Hao
> 
>


Re: Review Request 54729: SENTRY-1538: Create schema for storing HMS path change and Sentry permission change.

Posted by kalyan kumar kalvagadda <kk...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54729/#review160738
-----------------------------------------------------------


Ship it!




After addressing the comment.

- kalyan kumar kalvagadda


On Dec. 14, 2016, 2:35 a.m., Hao Hao wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54729/
> -----------------------------------------------------------
> 
> (Updated Dec. 14, 2016, 2:35 a.m.)
> 
> 
> Review request for sentry, Alexander Kolbasov, kalyan kumar kalvagadda, and Vamsee Yarlagadda.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> Create schema for storing HMS path change and Sentry permission change. It contains change ID, timestamp and the change event in JSON format.
> 
> 
> Diffs
> -----
> 
>   sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/PathsUpdate.java 7cfb3bf57bd1a425b07df6c08db31b9691dd17f5 
>   sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/PermissionsUpdate.java 98349232bc658c39791e58b64949ecb975fff7a0 
>   sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/Updateable.java 4dc3a0cebdff89ee2f9070e4d822a28dbd164c08 
>   sentry-hdfs/sentry-hdfs-common/src/test/java/org/apache/sentry/hdfs/TestPathsUpdate.java 53243b44329997e64ab70db5e5d8c3614ab974d6 
>   sentry-hdfs/sentry-hdfs-common/src/test/java/org/apache/sentry/hdfs/TestPermissionUpdate.java PRE-CREATION 
>   sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/UpdateForwarder.java ea1c8f62f300abd03206fdd9ec76fbf216fc6a2d 
>   sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/TestUpdateForwarder.java 315d4b3a80b9ab3ac8704ecbab81876f141423f1 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPathChange.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPermChange.java PRE-CREATION 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/package.jdo fb5470f635e03c762beb5bc2c6b06641b2476ce9 
> 
> Diff: https://reviews.apache.org/r/54729/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Hao Hao
> 
>