> On April 6, 2018, 8:58 a.m., David Radley wrote:
> > addons/models/0000-Area0/0010-base_model.json
> > Lines 4 (patched)
> > <https://reviews.apache.org/r/66478/diff/4/?file=1993312#file1993312line4>
> >
> >     I suggest it would be cleaner to have the atlas glossary entiries and 
> > relationships in a new model file called atlasglossary.json.

That sounds like a good idea. I can look into making that change


> On April 6, 2018, 8:58 a.m., David Radley wrote:
> > addons/models/0000-Area0/0010-base_model.json
> > Line 105 (original), 180 (patched)
> > <https://reviews.apache.org/r/66478/diff/4/?file=1993312#file1993312line180>
> >
> >     You are subtyping __internal for the 3 new entities. Open metadata 
> > GlossaryTerm and the other glossary entities are actually referencables. I 
> > suggest subtypeing Referenceable to __internalReferencable and then using  
> > __internalReferencable as the supertype for the 3 atlas entities you are 
> > adding. In this way you stay mappable to the open metadata types. I think 
> > the qualified name is important for these glossary entities.

Yes, internally the qualifiedName and guid requirement is enforced.


> On April 6, 2018, 8:58 a.m., David Radley wrote:
> > intg/src/main/java/org/apache/atlas/AtlasErrorCode.java
> > Lines 132 (patched)
> > <https://reviews.apache.org/r/66478/diff/4/?file=1993320#file1993320line132>
> >
> >     I cannot see this being used - or what the text has to do with glossary

There's a partial update call for all 3 types, which can be used to update 
primitive attributes. This exception is to used in the partial update flow.


> On April 6, 2018, 8:58 a.m., David Radley wrote:
> > intg/src/main/java/org/apache/atlas/model/glossary/relations/AtlasRelatedTermId.java
> > Lines 25 (patched)
> > <https://reviews.apache.org/r/66478/diff/4/?file=1993331#file1993331line25>
> >
> >     I am wondering why you need to introduce a new custom Atlas annotation

The JSON annotations are becoming repetitive across the intg module, idea is to 
use a meta annotation to control the JSON related features. Eventually all 
model classes will be annotated with this.


> On April 6, 2018, 8:58 a.m., David Radley wrote:
> > repository/src/main/java/org/apache/atlas/glossary/GlossaryService.java
> > Lines 576 (patched)
> > <https://reviews.apache.org/r/66478/diff/4/?file=1993337#file1993337line576>
> >
> >     why can't they be empty?

For creating a relation between a glossary and term both the GUIDs need to be 
known. Similarly for any other relation in the Glossary domain, the guid of 
both ends should be available in order to create a relation.


> On April 6, 2018, 8:58 a.m., David Radley wrote:
> > repository/src/main/java/org/apache/atlas/glossary/GlossaryService.java
> > Lines 590 (patched)
> > <https://reviews.apache.org/r/66478/diff/4/?file=1993337#file1993337line590>
> >
> >     Is this tracked by a Jira?

Not yet


> On April 6, 2018, 8:58 a.m., David Radley wrote:
> > repository/src/main/java/org/apache/atlas/glossary/GlossaryService.java
> > Lines 602 (patched)
> > <https://reviews.apache.org/r/66478/diff/4/?file=1993337#file1993337line602>
> >
> >     Unless it was created by the entity API and the relationship has not 
> > been created yet. I suggest the error mentions this - as the user would 
> > need to know this to be able to fix the issue.

Will add specialized errors for glossary, right now I was just using the 
generic bad request.


> On April 6, 2018, 8:58 a.m., David Radley wrote:
> > repository/src/main/java/org/apache/atlas/glossary/GlossaryService.java
> > Lines 618 (patched)
> > <https://reviews.apache.org/r/66478/diff/4/?file=1993337#file1993337line618>
> >
> >     Unless it was created by the entity API and the relationship has not 
> > been created yet. I suggest the error mentions this - as the user would 
> > need to know this to be able to fix the issue.

Same as above


> On April 6, 2018, 8:58 a.m., David Radley wrote:
> > repository/src/main/java/org/apache/atlas/glossary/GlossaryService.java
> > Lines 643 (patched)
> > <https://reviews.apache.org/r/66478/diff/4/?file=1993337#file1993337line643>
> >
> >     Is the a Jira for this

Not yet


> On April 6, 2018, 8:58 a.m., David Radley wrote:
> > repository/src/main/java/org/apache/atlas/glossary/GlossaryService.java
> > Lines 663 (patched)
> > <https://reviews.apache.org/r/66478/diff/4/?file=1993337#file1993337line663>
> >
> >     Is there a Jira for this?

No, I'll be collectively addressing these relation related TODOs in one single 
glossary enhancement JIRA


> On April 6, 2018, 8:58 a.m., David Radley wrote:
> > repository/src/main/java/org/apache/atlas/glossary/GlossaryService.java
> > Lines 678 (patched)
> > <https://reviews.apache.org/r/66478/diff/4/?file=1993337#file1993337line678>
> >
> >     I suspect you wanted to put some text here.

Good catch david.


> On April 6, 2018, 8:58 a.m., David Radley wrote:
> > repository/src/main/java/org/apache/atlas/repository/ogm/glossary/AtlasGlossaryCategoryDTO.java
> > Lines 18 (patched)
> > <https://reviews.apache.org/r/66478/diff/4/?file=1993344#file1993344line18>
> >
> >     what does the ogm package mean?

OGM notion is similar to ORM.


- Apoorv


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


On April 5, 2018, 11:57 p.m., Apoorv Naik wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66478/
> -----------------------------------------------------------
> 
> (Updated April 5, 2018, 11:57 p.m.)
> 
> 
> Review request for atlas, keval bhatt, Madhan Neethiraj, and Sarath 
> Subramanian.
> 
> 
> Bugs: ATLAS-2534
>     https://issues.apache.org/jira/browse/ATLAS-2534
> 
> 
> Repository: atlas
> 
> 
> Description
> -------
> 
> Glossary implementation, no UI
> 
> 
> Diffs
> -----
> 
>   addons/models/0000-Area0/0010-base_model.json aebe955db 
>   
> graphdb/api/src/main/java/org/apache/atlas/repository/graphdb/AtlasGraphQuery.java
>  7bdbeabf0 
>   
> graphdb/common/src/main/java/org/apache/atlas/repository/graphdb/tinkerpop/query/NativeTinkerpopGraphQuery.java
>  75665592e 
>   
> graphdb/common/src/main/java/org/apache/atlas/repository/graphdb/tinkerpop/query/TinkerpopGraphQuery.java
>  96b9705fa 
>   
> graphdb/common/src/main/java/org/apache/atlas/repository/graphdb/tinkerpop/query/expr/OrderByPredicate.java
>  PRE-CREATION 
>   
> graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/AtlasJanusGraph.java
>  6820a93c2 
>   
> graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/query/NativeJanusGraphQuery.java
>  d3c976df5 
>   
> graphdb/titan0/src/main/java/org/apache/atlas/repository/graphdb/titan0/query/NativeTitan0GraphQuery.java
>  2903ae228 
>   intg/src/main/java/org/apache/atlas/AtlasErrorCode.java 997ac68fb 
>   intg/src/main/java/org/apache/atlas/model/AtlasBaseModelObject.java 
> 688f6f4d2 
>   intg/src/main/java/org/apache/atlas/model/annotation/AtlasJSON.java 
> PRE-CREATION 
>   intg/src/main/java/org/apache/atlas/model/glossary/AtlasGlossary.java 
> PRE-CREATION 
>   
> intg/src/main/java/org/apache/atlas/model/glossary/AtlasGlossaryBaseObject.java
>  PRE-CREATION 
>   
> intg/src/main/java/org/apache/atlas/model/glossary/AtlasGlossaryCategory.java 
> PRE-CREATION 
>   intg/src/main/java/org/apache/atlas/model/glossary/AtlasGlossaryTerm.java 
> PRE-CREATION 
>   
> intg/src/main/java/org/apache/atlas/model/glossary/enums/AtlasTermAssignmentStatus.java
>  PRE-CREATION 
>   
> intg/src/main/java/org/apache/atlas/model/glossary/enums/AtlasTermRelationshipStatus.java
>  PRE-CREATION 
>   
> intg/src/main/java/org/apache/atlas/model/glossary/relations/AtlasGlossaryId.java
>  PRE-CREATION 
>   
> intg/src/main/java/org/apache/atlas/model/glossary/relations/AtlasRelatedCategoryId.java
>  PRE-CREATION 
>   
> intg/src/main/java/org/apache/atlas/model/glossary/relations/AtlasRelatedTermId.java
>  PRE-CREATION 
>   
> intg/src/main/java/org/apache/atlas/model/glossary/relations/AtlasTermAssignmentId.java
>  PRE-CREATION 
>   
> intg/src/main/java/org/apache/atlas/model/glossary/relations/AtlasTermCategorizationId.java
>  PRE-CREATION 
>   intg/src/main/java/org/apache/atlas/model/instance/AtlasEntity.java 
> 395431922 
>   intg/src/main/java/org/apache/atlas/model/instance/AtlasRelationship.java 
> 576847f6e 
>   intg/src/main/java/org/apache/atlas/type/AtlasRelationshipType.java 
> 61168f63f 
>   repository/src/main/java/org/apache/atlas/glossary/GlossaryService.java 
> PRE-CREATION 
>   
> repository/src/main/java/org/apache/atlas/repository/ogm/AbstractDataTransferObject.java
>  f1a8bc91d 
>   
> repository/src/main/java/org/apache/atlas/repository/ogm/AtlasSavedSearchDTO.java
>  a1a8f598d 
>   
> repository/src/main/java/org/apache/atlas/repository/ogm/AtlasUserProfileDTO.java
>  bcf2b9d27 
>   repository/src/main/java/org/apache/atlas/repository/ogm/DTORegistry.java 
> 818960d96 
>   repository/src/main/java/org/apache/atlas/repository/ogm/DataAccess.java 
> c99d2f836 
>   
> repository/src/main/java/org/apache/atlas/repository/ogm/glossary/AbstractGlossaryDTO.java
>  PRE-CREATION 
>   
> repository/src/main/java/org/apache/atlas/repository/ogm/glossary/AtlasGlossaryCategoryDTO.java
>  PRE-CREATION 
>   
> repository/src/main/java/org/apache/atlas/repository/ogm/glossary/AtlasGlossaryDTO.java
>  PRE-CREATION 
>   
> repository/src/main/java/org/apache/atlas/repository/ogm/glossary/AtlasGlossaryTermDTO.java
>  PRE-CREATION 
>   
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasEntityChangeNotifier.java
>  b9945d4fc 
>   
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasGraphUtilsV1.java
>  13ee2a6df 
>   
> repository/src/main/java/org/apache/atlas/repository/userprofile/UserProfileService.java
>  a428b92ee 
>   repository/src/test/java/org/apache/atlas/TestModules.java c901e891c 
>   repository/src/test/java/org/apache/atlas/glossary/GlossaryServiceTest.java 
> PRE-CREATION 
>   
> repository/src/test/java/org/apache/atlas/glossary/PaginationHelperTest.java 
> PRE-CREATION 
>   
> repository/src/test/java/org/apache/atlas/repository/impexp/ZipFileResourceTestUtils.java
>  d8e916d77 
>   
> repository/src/test/java/org/apache/atlas/repository/userprofile/UserProfileServiceTest.java
>  8e19f79e8 
>   
> webapp/src/main/java/org/apache/atlas/notification/EntityNotificationListenerV2.java
>  01a95cf80 
>   webapp/src/main/java/org/apache/atlas/web/rest/GlossaryREST.java 
> PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/66478/diff/4/
> 
> 
> Testing
> -------
> 
> Added test GlossaryServiceTest, runs successfully.
> 
> mvn clean package executes successfully.
> 
> PreCommit: 
> https://builds.apache.org/view/A/view/Atlas/job/PreCommit-ATLAS-Build-Test/235/
>  (in-progress)
> 
> 
> Thanks,
> 
> Apoorv Naik
> 
>

Reply via email to