atlas-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From David Radley <david...@apache.org>
Subject Re: Review Request 63503: ATLAS-1095 - Review of OCF - the Open Connector Framework
Date Thu, 30 Nov 2017 17:06:58 GMT

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




om-fwk-ocf/src/main/java/org/apache/atlas/ocf/ffdc/OCFErrorCode.java
Lines 100 (patched)
<https://reviews.apache.org/r/63503/#comment270358>

    Shouldn't this br iterator rather than enumeration



om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/Annotation.java
Lines 215 (patched)
<https://reviews.apache.org/r/63503/#comment270365>

    The name Annotationtype seems to me to indicate the formal type. I wonder if we could
have the work informal or display in the field name. Maybe setInformalTypeName or setDisplayName.



om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/Endpoint.java
Lines 60 (patched)
<https://reviews.apache.org/r/63503/#comment270373>

    The field name encrypted implies that it is a boolean. 
    I am not sure what might be put in the encrypted string. 
    For example I assume we might be often using TLS which starts with the client supplying
a list of supported cipher suites (ciphers and hash functions).   
    Or for JDBC https://docs.oracle.com/cd/B28359_01/java.111/b31224/clntsec.htm#EHAFHEIG.



om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/InformalTag.java
Lines 24 (patched)
<https://reviews.apache.org/r/63503/#comment270369>

    You mention tag here. This implies there is a concept of tag the user needs to know about.
    
    I wonder if we use tag here to mean informal classification or we just call this subjective
or informal or feedback Classification.



om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/InformalTag.java
Lines 53 (patched)
<https://reviews.apache.org/r/63503/#comment270370>

    In addition to the parentAsset, could we have the name and user on the constructor the
another  constructor with the optional description? We could reject creations with a null
name or invalid user.



om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/InformalTag.java
Lines 90 (patched)
<https://reviews.apache.org/r/63503/#comment270371>

    what code is responsible for the policing of the visibility?



om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/License.java
Lines 39 (patched)
<https://reviews.apache.org/r/63503/#comment270374>

    I suggest licensee to be consistent with the class name



om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/License.java
Lines 55 (patched)
<https://reviews.apache.org/r/63503/#comment270375>

    could the constructor normally be called with the mandatory fields it needs - including
Licensetype and licenseTypeGUID.



om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/License.java
Lines 251 (patched)
<https://reviews.apache.org/r/63503/#comment270376>

    I am wondering why the license conditions is plural but we only return a String not a
collection of some sort.
    
    Same question for notes.



om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/Location.java
Lines 31 (patched)
<https://reviews.apache.org/r/63503/#comment270377>

    How do we set up the location with different levels of granularity? For example longitute
latitude or countries, whcih would appear to be different structures in which location could
be specified



om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/MapSchemaElement.java
Lines 23 (patched)
<https://reviews.apache.org/r/63503/#comment270378>

    I googled range and domain in maths and it seems that domain would be the input to a function
and range the output. This matches the method javadoc. you example is that a domain might
be a property name and range a property value. I assume in this came there are schem elements
that describe property names and schema elements that would describe property values. 
    
    Is this likely to tbe the way this is used? I was thinking that the schema elements might
be elements in an XML schema or a database schema - as such I was not expecting property values.
I am thinking that this is a mapping between 2 metamodels - which does not obviously fit for
me with mapping property names and values. Have I misunderstood this?   
    
    I assume this mapping is a 1 to 1 mapping - not XSLT or XQuery - I wonder if we handle
those richer mappings.



om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/MapSchemaElement.java
Lines 109 (patched)
<https://reviews.apache.org/r/63503/#comment270379>

    ype => type



om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/Meaning.java
Lines 31 (patched)
<https://reviews.apache.org/r/63503/#comment270380>

    should be include the guid of the term here.



om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/Meaning.java
Lines 72 (patched)
<https://reviews.apache.org/r/63503/#comment270381>

    I suggest using meaning in the descriptions rather than glossary term - to be consistent
with the class name.



om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/Schema.java
Lines 51 (patched)
<https://reviews.apache.org/r/63503/#comment270383>

    I am wondering whetehr this is size.



om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/Schema.java
Lines 190 (patched)
<https://reviews.apache.org/r/63503/#comment270384>

    grpah => graph


- David Radley


On Nov. 10, 2017, 12:48 p.m., Mandy Chessell wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63503/
> -----------------------------------------------------------
> 
> (Updated Nov. 10, 2017, 12:48 p.m.)
> 
> 
> Review request for atlas and Madhan Neethiraj.
> 
> 
> Repository: atlas
> 
> 
> Description
> -------
> 
> This patch contains the open connector framework code.  This code is located in the om-fwk-ocf
component and is described in JIRA https://issues.apache.org/jira/browse/ATLAS-1095
> 
> I have added a new patch to the Jira with fixes from Yao's comments.  Upgraded Maven
to 352 and rebuilt/rerun tests.
> 
> 
> Diffs
> -----
> 
>   om-fwk-ocf/README.md PRE-CREATION 
>   om-fwk-ocf/pom.xml PRE-CREATION 
>   om-fwk-ocf/src/main/java/org/apache/atlas/ocf/Connector.java PRE-CREATION 
>   om-fwk-ocf/src/main/java/org/apache/atlas/ocf/ConnectorBase.java PRE-CREATION 
>   om-fwk-ocf/src/main/java/org/apache/atlas/ocf/ConnectorBroker.java PRE-CREATION 
>   om-fwk-ocf/src/main/java/org/apache/atlas/ocf/ConnectorProvider.java PRE-CREATION 
>   om-fwk-ocf/src/main/java/org/apache/atlas/ocf/ConnectorProviderBase.java PRE-CREATION

>   om-fwk-ocf/src/main/java/org/apache/atlas/ocf/ffdc/ConnectionCheckedException.java
PRE-CREATION 
>   om-fwk-ocf/src/main/java/org/apache/atlas/ocf/ffdc/ConnectorCheckedException.java PRE-CREATION

>   om-fwk-ocf/src/main/java/org/apache/atlas/ocf/ffdc/OCFCheckedExceptionBase.java PRE-CREATION

>   om-fwk-ocf/src/main/java/org/apache/atlas/ocf/ffdc/OCFErrorCode.java PRE-CREATION 
>   om-fwk-ocf/src/main/java/org/apache/atlas/ocf/ffdc/OCFRuntimeException.java PRE-CREATION

>   om-fwk-ocf/src/main/java/org/apache/atlas/ocf/ffdc/PropertyServerException.java PRE-CREATION

>   om-fwk-ocf/src/main/java/org/apache/atlas/ocf/ffdc/README.md PRE-CREATION 
>   om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/AdditionalProperties.java
PRE-CREATION 
>   om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/Analysis.java PRE-CREATION

>   om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/Annotation.java PRE-CREATION

>   om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/AnnotationStatus.java PRE-CREATION

>   om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/Annotations.java PRE-CREATION

>   om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/AssetDescriptor.java PRE-CREATION

>   om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/AssetDetail.java PRE-CREATION

>   om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/AssetPropertyBase.java PRE-CREATION

>   om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/AssetSummary.java PRE-CREATION

>   om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/AssetUniverse.java PRE-CREATION

>   om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/Certification.java PRE-CREATION

>   om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/Certifications.java PRE-CREATION

>   om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/Classification.java PRE-CREATION

>   om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/Classifications.java PRE-CREATION

>   om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/Comment.java PRE-CREATION

>   om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/CommentType.java PRE-CREATION

>   om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/Comments.java PRE-CREATION

>   om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/ConnectedAssetProperties.java
PRE-CREATION 
>   om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/Connection.java PRE-CREATION

>   om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/Connections.java PRE-CREATION

>   om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/ConnectorType.java PRE-CREATION

>   om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/DerivedSchemaElement.java
PRE-CREATION 
>   om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/ElementHeader.java PRE-CREATION

>   om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/ElementType.java PRE-CREATION

>   om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/EmbeddedConnection.java PRE-CREATION

>   om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/EmbeddedConnections.java PRE-CREATION

>   om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/Endpoint.java PRE-CREATION

>   om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/ExternalIdentifier.java PRE-CREATION

>   om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/ExternalIdentifiers.java PRE-CREATION

>   om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/ExternalReference.java PRE-CREATION

>   om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/ExternalReferences.java PRE-CREATION

>   om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/Feedback.java PRE-CREATION

>   om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/InformalTag.java PRE-CREATION

>   om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/InformalTags.java PRE-CREATION

>   om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/KeyPattern.java PRE-CREATION

>   om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/License.java PRE-CREATION

>   om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/Licenses.java PRE-CREATION

>   om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/Like.java PRE-CREATION 
>   om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/Likes.java PRE-CREATION 
>   om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/Lineage.java PRE-CREATION

>   om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/Location.java PRE-CREATION

>   om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/Locations.java PRE-CREATION

>   om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/MapSchemaElement.java PRE-CREATION

>   om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/Meaning.java PRE-CREATION

>   om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/Meanings.java PRE-CREATION

>   om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/Note.java PRE-CREATION 
>   om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/NoteLog.java PRE-CREATION

>   om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/NoteLogs.java PRE-CREATION

>   om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/Notes.java PRE-CREATION 
>   om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/PrimitiveSchemaElement.java
PRE-CREATION 
>   om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/PropertyBase.java PRE-CREATION

>   om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/Rating.java PRE-CREATION 
>   om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/Ratings.java PRE-CREATION

>   om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/Referenceable.java PRE-CREATION

>   om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/RelatedAsset.java PRE-CREATION

>   om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/RelatedAssetProperties.java
PRE-CREATION 
>   om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/RelatedAssets.java PRE-CREATION

>   om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/RelatedMediaReference.java
PRE-CREATION 
>   om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/RelatedMediaReferences.java
PRE-CREATION 
>   om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/RelatedMediaType.java PRE-CREATION

>   om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/RelatedMediaUsage.java PRE-CREATION

>   om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/Schema.java PRE-CREATION 
>   om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/SchemaAttribute.java PRE-CREATION

>   om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/SchemaAttributeGUIDs.java
PRE-CREATION 
>   om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/SchemaAttributes.java PRE-CREATION

>   om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/SchemaElement.java PRE-CREATION

>   om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/SchemaImplementationQueries.java
PRE-CREATION 
>   om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/SchemaImplementationQuery.java
PRE-CREATION 
>   om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/SchemaLink.java PRE-CREATION

>   om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/SchemaLinks.java PRE-CREATION

>   om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/StarRating.java PRE-CREATION

>   om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/VirtualConnection.java PRE-CREATION

>   om-fwk-ocf/src/test/java/org/apache/atlas/ocf/test/QuickTest.java PRE-CREATION 
>   om-fwk-ocf/src/test/java/org/apache/atlas/ocf/test/TestConnector.java PRE-CREATION

>   om-fwk-ocf/src/test/java/org/apache/atlas/ocf/test/TestConnectorProvider.java PRE-CREATION

> 
> 
> Diff: https://reviews.apache.org/r/63503/diff/2/
> 
> 
> Testing
> -------
> 
> Simple sniff test to create connectors.
> 
> 
> File Attachments
> ----------------
> 
> 0001-ATLAS-1095-initial-code-drop-for-OCF-with-fixes-from.patch
>   https://reviews.apache.org/media/uploaded/files/2017/11/10/ad9eae3c-bb68-4bdc-b6b9-62ba12f651a0__0001-ATLAS-1095-initial-code-drop-for-OCF-with-fixes-from.patch
> 
> 
> Thanks,
> 
> Mandy Chessell
> 
>


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