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

Reply via email to