atlas-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Yao Li <bertha...@126.com>
Subject Re: Review Request 65123: ATLAS-2298 - Review of OCF Database Connector
Date Tue, 30 Jan 2018 14:25:00 GMT


> On Jan. 18, 2018, 12:16 p.m., Graham Wallis wrote:
> > jdbc/connectors/JDBCConnection.java
> > Lines 84 (patched)
> > <https://reviews.apache.org/r/65123/diff/2/?file=1941456#file1941456line84>
> >
> >     Is it deliberate that we are testing the super-class attribute?
> 
> Yao Li wrote:
>     will fix this during the workshop. and this part also show in OMRS Connection.

this function is deleted now and we can get the securedProperties from OCFDatabaseConnector.
thanks.


> On Jan. 18, 2018, 12:16 p.m., Graham Wallis wrote:
> > jdbc/connectors/JDBCConnection.java
> > Lines 90 (patched)
> > <https://reviews.apache.org/r/65123/diff/2/?file=1941456#file1941456line90>
> >
> >     Do we need to go to the super class here?
> 
> Yao Li wrote:
>     will fix this during the workshop and this part also show in OMRS Connection

this function is deleted now and we can get the securedProperties from OCFDatabaseConnector.
thanks.


> On Jan. 18, 2018, 12:16 p.m., Graham Wallis wrote:
> > jdbc/connectors/JDBCConnectorBase.java
> > Lines 32 (patched)
> > <https://reviews.apache.org/r/65123/diff/2/?file=1941458#file1941458line32>
> >
> >     Can the connect() be called multiple times to generate multiple connections?
e.g. with a call to one of the setters between calls to connect(). I'm just wondering how
we'd intend to use the 'conn' member if that is the case.
> 
> Yao Li wrote:
>     the initial design is every time the application wants to execute the query, this
conn will be get again.

I will keep this one and discuss this with the front-end, how exactly will it use this.


> On Jan. 18, 2018, 12:16 p.m., Graham Wallis wrote:
> > jdbc/connectors/gaian/GaianJDBCConnector.java
> > Lines 63 (patched)
> > <https://reviews.apache.org/r/65123/diff/2/?file=1941460#file1941460line63>
> >
> >     You could use the string utils isEmpty() method - or a similar method - to test
for null-ness or emptiness.
> 
> Yao Li wrote:
>     use isEmpty() now and in apache commons it offers isNullorEmpty() for String. this
is also used by Manndy's code, we will talk about this during the workshop

we change the code to if("".equals(query))


> On Jan. 18, 2018, 12:16 p.m., Graham Wallis wrote:
> > jdbc/connectors/gaian/GaianJDBCConnector.java
> > Lines 186 (patched)
> > <https://reviews.apache.org/r/65123/diff/2/?file=1941460#file1941460line186>
> >
> >     There are ways to dynamically get the name of the enclosing method - but they
may have drawbacks (e.g. creation of inner class or additional runtime object) - but someone
much more expert than me may have a suggestion :-)
> 
> Yao Li wrote:
>     will talk about this through the workshop. thanks

because Atlas is trying to get rid of the stack traces, for now we will use simple method
name. but maybe in the Unit Test to test whether the methond name is correct.


> On Jan. 18, 2018, 12:16 p.m., Graham Wallis wrote:
> > jdbc/pom.xml
> > Lines 6 (patched)
> > <https://reviews.apache.org/r/65123/diff/2/?file=1941467#file1941467line6>
> >
> >     There should be a parent POM reference, which (depending on how nested this
module is) should look something like this:
> >     <parent>
> >             <artifactId>apache-atlas</artifactId>
> >             <groupId>org.apache.atlas</groupId>
> >             <version>1.0.0-SNAPSHOT</version>
> >     </parent>
> >     But check whether Mandy wants this nested under
> >     <artifactId>om-fwk-ocf</artifactId>
> 
> Yao Li wrote:
>     will fix this during the workshop on 30-01. thanks!

this will be an seperate module for now. Maybe later will be added toghther with other connectors.but
in the pom.xml we will add ocf as a denpendency


- Yao


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


On Jan. 29, 2018, 3:58 p.m., Yao Li wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65123/
> -----------------------------------------------------------
> 
> (Updated Jan. 29, 2018, 3:58 p.m.)
> 
> 
> Review request for atlas and Mandy Chessell.
> 
> 
> Repository: atlas
> 
> 
> Description
> -------
> 
> The OCF Database Connector is the subclass of OCF Connector and it is designed especially
for connection to database to retrieve data. 
> Here we implement a connector for Gaian (GaianOCFConnector) as an example for using OCF
Database Connector. It is related to Open Connector Framework. The JIRA can be found https://issues.apache.org/jira/browse/ATLAS-2298
> 
> 
> Diffs
> -----
> 
>   ocfdb/README.md  
>   ocfdb/pom.xml  
>   ocfdb/src/main/java/org/apache/atlas/ocfdb/connectors/OCFDataProvider.java  
>   ocfdb/src/main/java/org/apache/atlas/ocfdb/connectors/OCFDatabaseConnection.java  
>   ocfdb/src/main/java/org/apache/atlas/ocfdb/connectors/OCFDatabaseConnector.java  
>   ocfdb/src/main/java/org/apache/atlas/ocfdb/connectors/OCFDatabaseConnectorProviderBase.java
 
>   ocfdb/src/main/java/org/apache/atlas/ocfdb/connectors/gaian/GaianOCFConnector.java
 
>   ocfdb/src/main/java/org/apache/atlas/ocfdb/connectors/gaian/GaianOCFConnectorProvider.java
 
>   ocfdb/src/main/java/org/apache/atlas/ocfdb/ffdc/DatabaseAccessCheckedException.java
 
>   ocfdb/src/main/java/org/apache/atlas/ocfdb/ffdc/OCFDatabaseCheckedExceptionBase.java
 
>   ocfdb/src/main/java/org/apache/atlas/ocfdb/ffdc/OCFDatabaseConnectorErrorCode.java
 
>   ocfdb/src/main/java/org/apache/atlas/ocfdb/ffdc/OCFDatabaseConnectorRuntimeException.java
 
> 
> 
> Diff: https://reviews.apache.org/r/65123/diff/5/
> 
> 
> Testing
> -------
> 
> create an instance of the Connector and use getData() function. Gaian has to be set up
in advance.
> 
> 
> File Attachments
> ----------------
> 
> ATLAS-2298-update-patch-based-on-reviews.patch
>   https://reviews.apache.org/media/uploaded/files/2018/01/29/e228728b-3ac4-49a9-8b44-bb3ff2a04052__ATLAS-2298-update-patch-based-on-reviews.patch
> ATLAS-2298-update-patch-based-on-reviews.patch
>   https://reviews.apache.org/media/uploaded/files/2018/01/29/425dded1-e882-4f5b-902b-acea0acec74d__ATLAS-2298-update-patch-based-on-reviews.patch
> ATLAS-2298-update-the-patch-29-1.patch
>   https://reviews.apache.org/media/uploaded/files/2018/01/29/4c6d596c-dd34-4cf6-8fe0-33258e9965a6__ATLAS-2298-update-the-patch-29-1.patch
> ATLAS-2298-update-the-patch-29-1.patch
>   https://reviews.apache.org/media/uploaded/files/2018/01/29/db1c4085-05c8-41d7-9367-274e5dadfa1c__ATLAS-2298-update-the-patch-29-1.patch
> 
> 
> Thanks,
> 
> Yao Li
> 
>


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