You are viewing a plain text version of this content. The canonical link for it is here.

Posted to dev@atlas.apache.org by Apoorv Naik <na...@gmail.com> on 2017/04/26 19:09:35 UTC

Review Request 58746: ATLAS-1736: Type validations with special character handling

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

Review request for atlas.


Bugs: ATLAS-1736
    https://issues.apache.org/jira/browse/ATLAS-1736


Repository: atlas


Description
-------

Added empty attribute(s) check and dealing with special characters like $,%,{,} (reserved for special titan)


Diffs
-----

  repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasStructDefStoreV1.java f154555b 
  repository/src/main/java/org/apache/atlas/repository/typestore/GraphBackedTypeStore.java 7037d1eb 


Diff: https://reviews.apache.org/r/58746/diff/1/


Testing
-------

mvn clean package executes successfully


Thanks,

Apoorv Naik

Re: Review Request 58746: ATLAS-1736: Type validations with special character handling

Posted by Madhan Neethiraj <ma...@apache.org>.

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




repository/src/main/java/org/apache/atlas/repository/typestore/GraphBackedTypeStore.java
Line 153 (original), 138 (patched)
<https://reviews.apache.org/r/58746/#comment246182>

    Please review exising use of getPropertyKey() API, to ensure that encodePropertyKey() is not called multiple times. For example,  GraphBackedTypeStore.getEnumType():
    
            for (String value : values) {
                String valueProperty = getPropertyKey(typeName, value);
                enumValues.add(new EnumValue(value, GraphHelper.getSingleValuedProperty(vertex, valueProperty, Integer.class)));
    
    GraphHelper.getSingleValuedProperty() calls encodePropertyKey(valueProperty).


- Madhan Neethiraj


On April 26, 2017, 7:09 p.m., Apoorv Naik wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58746/
> -----------------------------------------------------------
> 
> (Updated April 26, 2017, 7:09 p.m.)
> 
> 
> Review request for atlas.
> 
> 
> Bugs: ATLAS-1736
>     https://issues.apache.org/jira/browse/ATLAS-1736
> 
> 
> Repository: atlas
> 
> 
> Description
> -------
> 
> Added empty attribute(s) check and dealing with special characters like $,%,{,} (reserved for special titan)
> 
> 
> Diffs
> -----
> 
>   repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasStructDefStoreV1.java f154555b 
>   repository/src/main/java/org/apache/atlas/repository/typestore/GraphBackedTypeStore.java 7037d1eb 
> 
> 
> Diff: https://reviews.apache.org/r/58746/diff/1/
> 
> 
> Testing
> -------
> 
> mvn clean package executes successfully
> 
> 
> Thanks,
> 
> Apoorv Naik
> 
>

Re: Review Request 58746: ATLAS-1736: Type validations with special character handling

Posted by Madhan Neethiraj <ma...@apache.org>.

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




repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasStructDefStoreV1.java
Lines 437 (patched)
<https://reviews.apache.org/r/58746/#comment246451>

    if attributeDef is a new attribute, currentStructDef.getAttribute(attributeDef.getName()) will return null and .getTypeName() on the return value will be NPE. Can you please review?


- Madhan Neethiraj


On April 28, 2017, 4:25 p.m., Apoorv Naik wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58746/
> -----------------------------------------------------------
> 
> (Updated April 28, 2017, 4:25 p.m.)
> 
> 
> Review request for atlas.
> 
> 
> Bugs: ATLAS-1730 and ATLAS-1736
>     https://issues.apache.org/jira/browse/ATLAS-1730
>     https://issues.apache.org/jira/browse/ATLAS-1736
> 
> 
> Repository: atlas
> 
> 
> Description
> -------
> 
> Added empty attribute(s) check and dealing with special characters like $,%,{,} (reserved for special titan)
> 
> 
> Diffs
> -----
> 
>   repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasStructDefStoreV1.java f154555b 
>   repository/src/main/java/org/apache/atlas/repository/typestore/GraphBackedTypeStore.java 7037d1eb 
>   repository/src/test/java/org/apache/atlas/repository/typestore/GraphBackedTypeStoreTest.java c08bb881 
> 
> 
> Diff: https://reviews.apache.org/r/58746/diff/2/
> 
> 
> Testing
> -------
> 
> mvn clean package executes successfully
> 
> 
> Thanks,
> 
> Apoorv Naik
> 
>

Re: Review Request 58746: Type validations with special character handling

Posted by Madhan Neethiraj <ma...@apache.org>.

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


Ship it!




Ship It!

- Madhan Neethiraj


On May 2, 2017, midnight, Apoorv Naik wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58746/
> -----------------------------------------------------------
> 
> (Updated May 2, 2017, midnight)
> 
> 
> Review request for atlas, Ashutosh Mestry, Madhan Neethiraj, Sarath Subramanian, and Suma Shivaprasad.
> 
> 
> Bugs: ATLAS-1730, ATLAS-1736 and ATLAS-1747
>     https://issues.apache.org/jira/browse/ATLAS-1730
>     https://issues.apache.org/jira/browse/ATLAS-1736
>     https://issues.apache.org/jira/browse/ATLAS-1747
> 
> 
> Repository: atlas
> 
> 
> Description
> -------
> 
> 1. Added empty attribute(s) check 
> 2. Dealing with special characters like $,%,{,} (titan treats these specially)
> 3. Update of attribute type is not allowed anymore
> 
> 
> Diffs
> -----
> 
>   repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasStructDefStoreV1.java 6f1b80c8 
>   repository/src/main/java/org/apache/atlas/repository/typestore/GraphBackedTypeStore.java 7037d1eb 
>   repository/src/test/java/org/apache/atlas/repository/typestore/GraphBackedTypeStoreTest.java c08bb881 
> 
> 
> Diff: https://reviews.apache.org/r/58746/diff/5/
> 
> 
> Testing
> -------
> 
> mvn clean package executes successfully
> 
> 
> Thanks,
> 
> Apoorv Naik
> 
>

Re: Review Request 58746: Type validations with special character handling

Posted by David Radley <da...@uk.ibm.com>.

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




repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasStructDefStoreV1.java
Line 418 (original), 425 (patched)
<https://reviews.apache.org/r/58746/#comment246580>

    I assume this would be valid to add a mandatory attribute, if there were no instances. 
    
    There needs to be a way to change the shape of an entity to add a mandatory field. Otherwise users would need to hack the graph store. 
    
    It would be worth tracking this requirement to change the shape an an entity to add a manditory attribute. I suggest that a change like this would mean we would need to version the new shape; so requests for the old and new shape could be honoured.
    
    Will we support adding a mandatory attribute to a deleted entitydef then making it active?


- David Radley


On May 2, 2017, midnight, Apoorv Naik wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58746/
> -----------------------------------------------------------
> 
> (Updated May 2, 2017, midnight)
> 
> 
> Review request for atlas, Ashutosh Mestry, Madhan Neethiraj, Sarath Subramanian, and Suma Shivaprasad.
> 
> 
> Bugs: ATLAS-1730, ATLAS-1736 and ATLAS-1747
>     https://issues.apache.org/jira/browse/ATLAS-1730
>     https://issues.apache.org/jira/browse/ATLAS-1736
>     https://issues.apache.org/jira/browse/ATLAS-1747
> 
> 
> Repository: atlas
> 
> 
> Description
> -------
> 
> 1. Added empty attribute(s) check 
> 2. Dealing with special characters like $,%,{,} (titan treats these specially)
> 3. Update of attribute type is not allowed anymore
> 
> 
> Diffs
> -----
> 
>   repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasStructDefStoreV1.java 6f1b80c8 
>   repository/src/main/java/org/apache/atlas/repository/typestore/GraphBackedTypeStore.java 7037d1eb 
>   repository/src/test/java/org/apache/atlas/repository/typestore/GraphBackedTypeStoreTest.java c08bb881 
> 
> 
> Diff: https://reviews.apache.org/r/58746/diff/5/
> 
> 
> Testing
> -------
> 
> mvn clean package executes successfully
> 
> 
> Thanks,
> 
> Apoorv Naik
> 
>