atlas-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Madhan Neethiraj <mad...@apache.org>
Subject Re: Review Request 58985: Type deletion should invalidate property key in titan
Date Thu, 04 May 2017 21:01:51 GMT

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




graphdb/api/src/main/java/org/apache/atlas/repository/graphdb/AtlasGraphManagement.java
Lines 71 (patched)
<https://reviews.apache.org/r/58985/#comment247034>

    - arguments propertyClass and cardinality seem to be unused in the implementations. If
not needed, please remove
    - consider renaming 'propertyName' as 'propertyKey' - to be consistent with Titan API
usage



graphdb/titan0/src/main/java/org/apache/atlas/repository/graphdb/titan0/Titan0GraphManagement.java
Lines 123 (patched)
<https://reviews.apache.org/r/58985/#comment247029>

    "==" ==> "!=" ?
    
    Consider the following to make this simpler/cleaner:
    
      for (int i = 0; ; i++) {
        String deletedKeyName = propertyKey + "_deleted_" + i;
      
        if (management.getPropertyKey(deletedKeyName) == null) {
          management.changeName(propertyKey, deletedKeyName);
        
          break;
        }
      }
    
    Same for Titan1GraphManagement.java as well.



repository/src/main/java/org/apache/atlas/repository/graph/GraphBackedSearchIndexer.java
Lines 690 (patched)
<https://reviews.apache.org/r/58985/#comment247031>

    Don't we have propertKey's for map/array/entity-type attributes as well? If yes, propertyKey
for such attribute should be deleted as well. Please review.



repository/src/main/java/org/apache/atlas/repository/graph/GraphBackedSearchIndexer.java
Lines 706 (patched)
<https://reviews.apache.org/r/58985/#comment247039>

    Shouldn't invalidatePropertyKey() be called, irrespective of whether the attribute is
indexable or not?
    
    To avoid such confusion, consider renaming the method - like "deletePropertyKey()"


- Madhan Neethiraj


On May 4, 2017, 4:26 p.m., Apoorv Naik wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58985/
> -----------------------------------------------------------
> 
> (Updated May 4, 2017, 4:26 p.m.)
> 
> 
> Review request for atlas.
> 
> 
> Repository: atlas
> 
> 
> Description
> -------
> 
> This change supports the use-case where the user creates a type and deletes it sometime
later, now the redefinition of the type is restricted to use the same data type for the attributes
used during the initial creation. 
> 
> Solution: Rename the propertyKey corresponding to that attribute using the titan management
API by suffing _deleted_xxx where xxx is an increasing integer sequence starting from 0. The
reason for increment of xxx is that the type create, delete and re-create can be done multiple
times without any conflicting key in Titan.
> 
> 
> Diffs
> -----
> 
>   graphdb/api/src/main/java/org/apache/atlas/repository/graphdb/AtlasGraphManagement.java
5efd7c0f 
>   graphdb/titan0/src/main/java/org/apache/atlas/repository/graphdb/titan0/Titan0GraphManagement.java
9a17d9ee 
>   graphdb/titan1/src/main/java/org/apache/atlas/repository/graphdb/titan1/Titan1GraphManagement.java
12faeda5 
>   intg/src/main/java/org/apache/atlas/listener/TypeDefChangeListener.java 124f106f 
>   intg/src/main/java/org/apache/atlas/type/AtlasTypeRegistry.java ea2a7038 
>   repository/src/main/java/org/apache/atlas/repository/graph/GraphBackedSearchIndexer.java
47dccf19 
>   repository/src/test/java/org/apache/atlas/repository/impexp/ExportServiceTest.java
a7fc24cf 
>   repository/src/test/java/org/apache/atlas/repository/store/graph/AtlasTypeDefGraphStoreTest.java
84ad72c8 
> 
> 
> Diff: https://reviews.apache.org/r/58985/diff/1/
> 
> 
> Testing
> -------
> 
> mvn clean package and mvn clean install run successfully.
> 
> 
> Thanks,
> 
> Apoorv Naik
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message