Mailing-List: contact dev-help@atlas.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@atlas.apache.org
Content-Type: multipart/alternative;
 boundary="===============7436429004707534304=="
MIME-Version: 1.0
Subject: Re: Review Request 63503: ATLAS-1095 - Review of OCF - the Open
 Connector Framework
From: Mandy Chessell <mandy_chessell@uk.ibm.com>
To: Madhan Neethiraj <madhan@apache.org>
Cc: Mandy Chessell <mandy_chessell@uk.ibm.com>, Nigel Jones <jonesn@uk.ibm.com>,
 atlas <dev@atlas.incubator.apache.org>, David Radley <davidrad@apache.org>,
 Yao Li <bertha_22@126.com>
Date: Mon, 04 Dec 2017 18:49:20 -0000
Message-ID: <20171204184920.13537.82911@reviews-vm2.apache.org>
Auto-Submitted: auto-generated
Sender: Mandy Chessell <noreply@reviews.apache.org>
References: <20171201112458.34997.1744@reviews-vm2.apache.org>
In-Reply-To: <20171201112458.34997.1744@reviews-vm2.apache.org>
Reply-To: Mandy Chessell <mandy_chessell@uk.ibm.com>
archived-at: Mon, 04 Dec 2017 18:49:28 -0000

--===============7436429004707534304==
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit


> On Dec. 1, 2017, 11:24 a.m., David Radley wrote:
> > om-fwk-ocf/src/main/java/org/apache/atlas/ocf/properties/Connection.java
> > Lines 77 (patched)
> > <https://reviews.apache.org/r/63503/diff/2/?file=1887953#file1887953line77>
> >
> >     I think we need additionalProperties and securedProperties in line with the class java doc.
> 
> Mandy Chessell wrote:
>     Please could you explain where it is not in alignment.
> 
> David Radley wrote:
>     I see the comment with :
>      Other properties for the connection include:
>      *
>      *  ** type
>      *
>      *  ** description - A full description of the connection covering details of the assets it connects to
>      *  along with usage and versioning information.
>      *
>      *  ** additionalProperties - Any additional properties associated with the connection.
>      *
>      *  ** securedProperties - Protected properties for secure log on by connector to back end server.
>      *
>      *  ** connectorType - Properties that describe the connector type for the connector.
>      *
>      *  ** endpoint - Properties that describe the server endpoint where the connector will retrieve the assets.
>      *
>     
>     For me this implies there would be 2 sets of properties additionalProperties and securedProperties. but the java shows one field using both of these property names:
>      protected AdditionalProperties    securedProperties = null;
>      
>      From the javadoc the securedProperties seem to be related to log on, I was thinking the additional properties are extra properties that a metadata repository might have added and we want to maintain. Did I interpret this correctly?

Now I am confused - there are 2 variables - one for additional properties and one for secured properties.  The additionalproperties are located in Referenceable.  I have checked that Connection inherits from Referenceable.  

Are you suggesting that I duplicate the AdditionalProperties class so to create an other class called SecuredProperties so that they each have different class names that match the variable name - this seems wasteful.  Or are you suggesting that I remove the mention of AdditionalProperties in the header description and rely on people working it out from following the inheritance structure.  I do not think the first option is a good idea.  I think the second one is not ideal but I will do it if it makes you happier.  I would also need to delete the description of qualified name too as that is also in Referenceable but is not causing so much of an issue.


- Mandy


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


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
> 
>


--===============7436429004707534304==--