You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@atlas.apache.org by Yao Li <be...@126.com> on 2018/02/01 14:04:09 UTC

Re: Review Request 65435: ATLAS-2298 - Review of OCF Database Connector_New

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

(Updated Feb. 1, 2018, 2:04 p.m.)


Review request for atlas and Mandy Chessell.


Repository: atlas


Description
-------

This is the new review request for ATLAS-2298 OCF Database Connector. The old review on [https://reviews.apache.org/r/65123/](https://reviews.apache.org/r/65123/) will not be updated anymore.

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

  ocf-database-connector/README.md PRE-CREATION 
  ocf-database-connector/pom.xml PRE-CREATION 
  ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/connectors/OCFDatabaseConnection.java PRE-CREATION 
  ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/connectors/OCFDatabaseConnector.java PRE-CREATION 
  ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/connectors/OCFDatabaseConnectorProviderBase.java PRE-CREATION 
  ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/connectors/OCFDatabaseDataManager.java PRE-CREATION 
  ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/connectors/gaian/GaianOCFConnector.java PRE-CREATION 
  ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/connectors/gaian/GaianOCFConnectorProvider.java PRE-CREATION 
  ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/ffdc/DatabaseAccessCheckedException.java PRE-CREATION 
  ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/ffdc/OCFDatabaseCheckedExceptionBase.java PRE-CREATION 
  ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/ffdc/OCFDatabaseConnectorErrorCode.java PRE-CREATION 
  ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/ffdc/OCFDatabaseConnectorRuntimeException.java PRE-CREATION 
  pom.xml c15e0dad 


Diff: https://reviews.apache.org/r/65435/diff/1/


Testing (updated)
-------

see test folder.
Gaian has to set up in advance


Thanks,

Yao Li


Re: Review Request 65435: ATLAS-2298 - Review of OCF Database Connector_New

Posted by Yao Li <be...@126.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65435/
-----------------------------------------------------------

(Updated March 30, 2018, 9:33 a.m.)


Review request for atlas and Mandy Chessell.


Repository: atlas


Description
-------

This is the new review request for ATLAS-2298 OCF Database Connector. The old review on [https://reviews.apache.org/r/65123/](https://reviews.apache.org/r/65123/) will not be updated anymore.

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 (updated)
-----

  ocf-database-connector/README.md PRE-CREATION 
  ocf-database-connector/pom.xml PRE-CREATION 
  ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/connectors/OCFDatabaseConnection.java PRE-CREATION 
  ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/connectors/OCFDatabaseConnector.java PRE-CREATION 
  ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/connectors/OCFDatabaseConnectorProviderBase.java PRE-CREATION 
  ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/connectors/gaian/GaianOCFConnector.java PRE-CREATION 
  ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/connectors/gaian/GaianOCFConnectorProvider.java PRE-CREATION 
  ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/ffdc/DatabaseConnectCheckedException.java PRE-CREATION 
  ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/ffdc/OCFDatabaseCheckedExceptionBase.java PRE-CREATION 
  ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/ffdc/OCFDatabaseConnectorErrorCode.java PRE-CREATION 
  ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/ffdc/OCFDatabaseConnectorRuntimeException.java PRE-CREATION 
  ocf-database-connector/src/test/java/gaianocfconnector/GaianOCFConnectorTest.java PRE-CREATION 
  ocf-database-connector/src/test/java/gaianocfconnector/UseGaianOCFConnector.java PRE-CREATION 
  pom.xml ee2746a905f84359d426acfb1fa6d64c6310aaaf 


Diff: https://reviews.apache.org/r/65435/diff/5/

Changes: https://reviews.apache.org/r/65435/diff/4-5/


Testing
-------

see test folder.
Gaian has to set up in advance


File Attachments
----------------

0005-ATLAS-2298-05-Feb-code-review.patch
  https://reviews.apache.org/media/uploaded/files/2018/02/05/292c1917-9deb-4ced-8bd6-1689fc45fbd4__0005-ATLAS-2298-05-Feb-code-review.patch


Thanks,

Yao Li


Re: Review Request 65435: ATLAS-2298 - Review of OCF Database Connector_New

Posted by Yao Li <be...@126.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65435/
-----------------------------------------------------------

(Updated March 29, 2018, 3:08 p.m.)


Review request for atlas and Mandy Chessell.


Repository: atlas


Description
-------

This is the new review request for ATLAS-2298 OCF Database Connector. The old review on [https://reviews.apache.org/r/65123/](https://reviews.apache.org/r/65123/) will not be updated anymore.

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 (updated)
-----

  ocf-database-connector/README.md PRE-CREATION 
  ocf-database-connector/pom.xml PRE-CREATION 
  ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/connectors/OCFDatabaseConnection.java PRE-CREATION 
  ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/connectors/OCFDatabaseConnector.java PRE-CREATION 
  ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/connectors/OCFDatabaseConnectorProviderBase.java PRE-CREATION 
  ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/connectors/gaian/GaianOCFConnector.java PRE-CREATION 
  ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/connectors/gaian/GaianOCFConnectorProvider.java PRE-CREATION 
  ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/ffdc/DatabaseConnectCheckedException.java PRE-CREATION 
  ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/ffdc/OCFDatabaseCheckedExceptionBase.java PRE-CREATION 
  ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/ffdc/OCFDatabaseConnectorErrorCode.java PRE-CREATION 
  ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/ffdc/OCFDatabaseConnectorRuntimeException.java PRE-CREATION 
  ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/util/PropertiesHelper.java PRE-CREATION 
  ocf-database-connector/src/main/resources/gaian.properties PRE-CREATION 
  ocf-database-connector/src/test/java/gaianocfconnector/GaianOCFConnectorTest.java PRE-CREATION 
  ocf-database-connector/src/test/java/gaianocfconnector/UseGaianOCFConnector.java PRE-CREATION 
  pom.xml ee2746a905f84359d426acfb1fa6d64c6310aaaf 


Diff: https://reviews.apache.org/r/65435/diff/4/

Changes: https://reviews.apache.org/r/65435/diff/3-4/


Testing
-------

see test folder.
Gaian has to set up in advance


File Attachments
----------------

0005-ATLAS-2298-05-Feb-code-review.patch
  https://reviews.apache.org/media/uploaded/files/2018/02/05/292c1917-9deb-4ced-8bd6-1689fc45fbd4__0005-ATLAS-2298-05-Feb-code-review.patch


Thanks,

Yao Li


Re: Review Request 65435: ATLAS-2298 - Review of OCF Database Connector_New

Posted by Yao Li <be...@126.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65435/
-----------------------------------------------------------

(Updated March 29, 2018, 2:33 p.m.)


Review request for atlas and Mandy Chessell.


Changes
-------

update code based on reviews


Repository: atlas


Description
-------

This is the new review request for ATLAS-2298 OCF Database Connector. The old review on [https://reviews.apache.org/r/65123/](https://reviews.apache.org/r/65123/) will not be updated anymore.

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 (updated)
-----

  ocf-database-connector/README.md PRE-CREATION 
  ocf-database-connector/pom.xml PRE-CREATION 
  ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/connectors/OCFDatabaseConnection.java PRE-CREATION 
  ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/connectors/OCFDatabaseConnector.java PRE-CREATION 
  ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/connectors/OCFDatabaseConnectorProviderBase.java PRE-CREATION 
  ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/connectors/gaian/GaianOCFConnector.java PRE-CREATION 
  ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/connectors/gaian/GaianOCFConnectorProvider.java PRE-CREATION 
  ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/ffdc/DatabaseConnectCheckedException.java PRE-CREATION 
  ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/ffdc/OCFDatabaseCheckedExceptionBase.java PRE-CREATION 
  ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/ffdc/OCFDatabaseConnectorErrorCode.java PRE-CREATION 
  ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/ffdc/OCFDatabaseConnectorRuntimeException.java PRE-CREATION 
  ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/util/PropertiesHelper.java PRE-CREATION 
  ocf-database-connector/src/main/resources/gaian.properties PRE-CREATION 
  ocf-database-connector/src/test/java/gaianocfconnector/GaianOCFConnectorTest.java PRE-CREATION 
  ocf-database-connector/src/test/java/gaianocfconnector/UseGaianOCFConnector.java PRE-CREATION 
  pom.xml ee2746a905f84359d426acfb1fa6d64c6310aaaf 


Diff: https://reviews.apache.org/r/65435/diff/3/

Changes: https://reviews.apache.org/r/65435/diff/2-3/


Testing
-------

see test folder.
Gaian has to set up in advance


File Attachments
----------------

0005-ATLAS-2298-05-Feb-code-review.patch
  https://reviews.apache.org/media/uploaded/files/2018/02/05/292c1917-9deb-4ced-8bd6-1689fc45fbd4__0005-ATLAS-2298-05-Feb-code-review.patch


Thanks,

Yao Li


Re: Review Request 65435: ATLAS-2298 - Review of OCF Database Connector_New

Posted by Yao Li <be...@126.com>.

> On March 26, 2018, 2 p.m., David Radley wrote:
> > ocf-database-connector/src/main/resources/gaian.properties
> > Lines 6 (patched)
> > <https://reviews.apache.org/r/65435/diff/2/?file=1985292#file1985292line6>
> >
> >     It does not look right to include default passwords here, without talking about poc / dev scenarios and not production scenarios
> 
> Yao Li wrote:
>     for now I am still not sure where do we store the information for the front-end Gaian. For now I just set it up as a properties file. I dont know these password will be stored in a central properties file or also OCF Connection for every asset. I will keep this issue open.

I have deleted the properties file for the new patch. for now if we want to use Gaian, The front-end application have to know where the Gaian is. GaianOCFConnector offer a constructor which contains all the set up for connecting to Gaian.


- Yao


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


On March 30, 2018, 9:33 a.m., Yao Li wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65435/
> -----------------------------------------------------------
> 
> (Updated March 30, 2018, 9:33 a.m.)
> 
> 
> Review request for atlas and Mandy Chessell.
> 
> 
> Repository: atlas
> 
> 
> Description
> -------
> 
> This is the new review request for ATLAS-2298 OCF Database Connector. The old review on [https://reviews.apache.org/r/65123/](https://reviews.apache.org/r/65123/) will not be updated anymore.
> 
> 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
> -----
> 
>   ocf-database-connector/README.md PRE-CREATION 
>   ocf-database-connector/pom.xml PRE-CREATION 
>   ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/connectors/OCFDatabaseConnection.java PRE-CREATION 
>   ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/connectors/OCFDatabaseConnector.java PRE-CREATION 
>   ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/connectors/OCFDatabaseConnectorProviderBase.java PRE-CREATION 
>   ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/connectors/gaian/GaianOCFConnector.java PRE-CREATION 
>   ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/connectors/gaian/GaianOCFConnectorProvider.java PRE-CREATION 
>   ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/ffdc/DatabaseConnectCheckedException.java PRE-CREATION 
>   ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/ffdc/OCFDatabaseCheckedExceptionBase.java PRE-CREATION 
>   ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/ffdc/OCFDatabaseConnectorErrorCode.java PRE-CREATION 
>   ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/ffdc/OCFDatabaseConnectorRuntimeException.java PRE-CREATION 
>   ocf-database-connector/src/test/java/gaianocfconnector/GaianOCFConnectorTest.java PRE-CREATION 
>   ocf-database-connector/src/test/java/gaianocfconnector/UseGaianOCFConnector.java PRE-CREATION 
>   pom.xml ee2746a905f84359d426acfb1fa6d64c6310aaaf 
> 
> 
> Diff: https://reviews.apache.org/r/65435/diff/5/
> 
> 
> Testing
> -------
> 
> see test folder.
> Gaian has to set up in advance
> 
> 
> File Attachments
> ----------------
> 
> 0005-ATLAS-2298-05-Feb-code-review.patch
>   https://reviews.apache.org/media/uploaded/files/2018/02/05/292c1917-9deb-4ced-8bd6-1689fc45fbd4__0005-ATLAS-2298-05-Feb-code-review.patch
> 
> 
> Thanks,
> 
> Yao Li
> 
>


Re: Review Request 65435: ATLAS-2298 - Review of OCF Database Connector_New

Posted by David Radley <da...@apache.org>.

> On March 26, 2018, 2 p.m., David Radley wrote:
> > ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/connectors/gaian/GaianOCFConnector.java
> > Lines 212 (patched)
> > <https://reviews.apache.org/r/65435/diff/2/?file=1985285#file1985285line212>
> >
> >     should we return this for an error?
> 
> Yao Li wrote:
>     i am not quite sure what do you mean

the finally is clause in java is driven in all cases - i.e even if you throw an error. I suggest this be moved to the end of the try block


> On March 26, 2018, 2 p.m., David Radley wrote:
> > ocf-database-connector/src/main/resources/gaian.properties
> > Lines 6 (patched)
> > <https://reviews.apache.org/r/65435/diff/2/?file=1985292#file1985292line6>
> >
> >     It does not look right to include default passwords here, without talking about poc / dev scenarios and not production scenarios
> 
> Yao Li wrote:
>     for now I am still not sure where do we store the information for the front-end Gaian. For now I just set it up as a properties file. I dont know these password will be stored in a central properties file or also OCF Connection for every asset. I will keep this issue open.
> 
> Yao Li wrote:
>     I have deleted the properties file for the new patch. for now if we want to use Gaian, The front-end application have to know where the Gaian is. GaianOCFConnector offer a constructor which contains all the set up for connecting to Gaian.

Thanks Yau -  I will wait for the new patch to be uploaded to Jira and into the review board. Then please close out the issues - when you feel they are addressed.


- David


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


On March 30, 2018, 9:33 a.m., Yao Li wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65435/
> -----------------------------------------------------------
> 
> (Updated March 30, 2018, 9:33 a.m.)
> 
> 
> Review request for atlas and Mandy Chessell.
> 
> 
> Repository: atlas
> 
> 
> Description
> -------
> 
> This is the new review request for ATLAS-2298 OCF Database Connector. The old review on [https://reviews.apache.org/r/65123/](https://reviews.apache.org/r/65123/) will not be updated anymore.
> 
> 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
> -----
> 
>   ocf-database-connector/README.md PRE-CREATION 
>   ocf-database-connector/pom.xml PRE-CREATION 
>   ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/connectors/OCFDatabaseConnection.java PRE-CREATION 
>   ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/connectors/OCFDatabaseConnector.java PRE-CREATION 
>   ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/connectors/OCFDatabaseConnectorProviderBase.java PRE-CREATION 
>   ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/connectors/gaian/GaianOCFConnector.java PRE-CREATION 
>   ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/connectors/gaian/GaianOCFConnectorProvider.java PRE-CREATION 
>   ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/ffdc/DatabaseConnectCheckedException.java PRE-CREATION 
>   ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/ffdc/OCFDatabaseCheckedExceptionBase.java PRE-CREATION 
>   ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/ffdc/OCFDatabaseConnectorErrorCode.java PRE-CREATION 
>   ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/ffdc/OCFDatabaseConnectorRuntimeException.java PRE-CREATION 
>   ocf-database-connector/src/test/java/gaianocfconnector/GaianOCFConnectorTest.java PRE-CREATION 
>   ocf-database-connector/src/test/java/gaianocfconnector/UseGaianOCFConnector.java PRE-CREATION 
>   pom.xml ee2746a905f84359d426acfb1fa6d64c6310aaaf 
> 
> 
> Diff: https://reviews.apache.org/r/65435/diff/5/
> 
> 
> Testing
> -------
> 
> see test folder.
> Gaian has to set up in advance
> 
> 
> File Attachments
> ----------------
> 
> 0005-ATLAS-2298-05-Feb-code-review.patch
>   https://reviews.apache.org/media/uploaded/files/2018/02/05/292c1917-9deb-4ced-8bd6-1689fc45fbd4__0005-ATLAS-2298-05-Feb-code-review.patch
> 
> 
> Thanks,
> 
> Yao Li
> 
>


Re: Review Request 65435: ATLAS-2298 - Review of OCF Database Connector_New

Posted by Yao Li <be...@126.com>.

> On March 26, 2018, 2 p.m., David Radley wrote:
> > ocf-database-connector/README.md
> > Lines 40 (patched)
> > <https://reviews.apache.org/r/65435/diff/2/?file=1985280#file1985280line40>
> >
> >     bad eol character

fixed, thanks


> On March 26, 2018, 2 p.m., David Radley wrote:
> > ocf-database-connector/README.md
> > Lines 42 (patched)
> > <https://reviews.apache.org/r/65435/diff/2/?file=1985280#file1985280line42>
> >
> >     typo sequene
> >     
> >     the sentence mentions we and you - I suggest using one or the other.

fixed, thanks


> On March 26, 2018, 2 p.m., David Radley wrote:
> > ocf-database-connector/README.md
> > Lines 58 (patched)
> > <https://reviews.apache.org/r/65435/diff/2/?file=1985280#file1985280line58>
> >
> >     bad eol character

this is for the code to show up propertly in markdown. I will leave like this.


> On March 26, 2018, 2 p.m., David Radley wrote:
> > ocf-database-connector/README.md
> > Lines 61 (patched)
> > <https://reviews.apache.org/r/65435/diff/2/?file=1985280#file1985280line61>
> >
> >     I thought impersonation would require a patch on top of Gaian. If this is the case we need to detail where to get this patch for impersonation to be able to be used here.

for now, Nigel add "proxy_user" and "proxy_password" in jdbc url. so there is no patch.Maybe in the future we will change the way we implement impersonation.


> On March 26, 2018, 2 p.m., David Radley wrote:
> > ocf-database-connector/README.md
> > Lines 69 (patched)
> > <https://reviews.apache.org/r/65435/diff/2/?file=1985280#file1985280line69>
> >
> >     same

this is for the code to show up propertly in markdown. I will leave like this.


> On March 26, 2018, 2 p.m., David Radley wrote:
> > ocf-database-connector/README.md
> > Lines 71 (patched)
> > <https://reviews.apache.org/r/65435/diff/2/?file=1985280#file1985280line71>
> >
> >     'here' is an extra word
> >     Typo Gian.
> >     We have not mention LDAP, I think we need to be careful mentioning products that might not be installed.
> >     We should not mention default passwords here - as in production this would not make sense.

deleted, thanks


> On March 26, 2018, 2 p.m., David Radley wrote:
> > ocf-database-connector/README.md
> > Lines 79 (patched)
> > <https://reviews.apache.org/r/65435/diff/2/?file=1985280#file1985280line79>
> >
> >     This text is assuming that people have access to a Gaian Ranger plugin. This is not currently available.
> >     
> >     You say "it is better to use **table function**. My understanding is that using the Ranger plugin requires the table function to be used. 
> >     
> >     You say "You need to change cst to your own table name". What is cst?

for the code now, we need to use table function to work together with Ranger plugin and Gaian. I deleted this totally.


> On March 26, 2018, 2 p.m., David Radley wrote:
> > ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/connectors/gaian/GaianOCFConnector.java
> > Lines 35 (patched)
> > <https://reviews.apache.org/r/65435/diff/2/?file=1985285#file1985285line35>
> >
> >     incomplete sentence

fixed, thanks


> On March 26, 2018, 2 p.m., David Radley wrote:
> > ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/connectors/gaian/GaianOCFConnector.java
> > Lines 159 (patched)
> > <https://reviews.apache.org/r/65435/diff/2/?file=1985285#file1985285line159>
> >
> >     finally is driven for errors as well - so this debug message is not correct. I suggst moving this debug to the end of the try section.

fixed, thanks


> On March 26, 2018, 2 p.m., David Radley wrote:
> > ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/connectors/gaian/GaianOCFConnector.java
> > Lines 212 (patched)
> > <https://reviews.apache.org/r/65435/diff/2/?file=1985285#file1985285line212>
> >
> >     should we return this for an error?

i am not quite sure what do you mean


> On March 26, 2018, 2 p.m., David Radley wrote:
> > ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/connectors/gaian/GaianOCFConnector.java
> > Lines 232 (patched)
> > <https://reviews.apache.org/r/65435/diff/2/?file=1985285#file1985285line232>
> >
> >     this if and body is a repeat of an previous if.

good spot, fixed,thanks


> On March 26, 2018, 2 p.m., David Radley wrote:
> > ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/ffdc/OCFDatabaseConnectorErrorCode.java
> > Lines 76 (patched)
> > <https://reviews.apache.org/r/65435/diff/2/?file=1985289#file1985289line76>
> >
> >     what is the user's valid information and valid use of OCF Connectors?

user information will be user id. Here should not check user's information. change it to "please check the query and connection to Gaian"


> On March 26, 2018, 2 p.m., David Radley wrote:
> > ocf-database-connector/src/main/resources/gaian.properties
> > Lines 6 (patched)
> > <https://reviews.apache.org/r/65435/diff/2/?file=1985292#file1985292line6>
> >
> >     It does not look right to include default passwords here, without talking about poc / dev scenarios and not production scenarios

for now I am still not sure where do we store the information for the front-end Gaian. For now I just set it up as a properties file. I dont know these password will be stored in a central properties file or also OCF Connection for every asset. I will keep this issue open.


> On March 26, 2018, 2 p.m., David Radley wrote:
> > ocf-database-connector/src/test/java/gaianocfconnector/GaianOCFConnectorTest.java
> > Lines 56 (patched)
> > <https://reviews.apache.org/r/65435/diff/2/?file=1985293#file1985293line56>
> >
> >     It looks like this class might a be unit test, but it seems that it requires Gaian to be running. If this cannot be run as a unit test - I suggest including instructions on how to run the tests in this file.

add instructions, thanks


> On March 26, 2018, 2 p.m., David Radley wrote:
> > ocf-database-connector/src/test/java/gaianocfconnector/UseGaianOCFConnector.java
> > Lines 33 (patched)
> > <https://reviews.apache.org/r/65435/diff/2/?file=1985294#file1985294line33>
> >
> >     I suggest the test throws the excpetions rather than gobbles them. In this way the test fails if there is an error

fixed, thanks


- Yao


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


On March 22, 2018, 11:23 a.m., Yao Li wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65435/
> -----------------------------------------------------------
> 
> (Updated March 22, 2018, 11:23 a.m.)
> 
> 
> Review request for atlas and Mandy Chessell.
> 
> 
> Repository: atlas
> 
> 
> Description
> -------
> 
> This is the new review request for ATLAS-2298 OCF Database Connector. The old review on [https://reviews.apache.org/r/65123/](https://reviews.apache.org/r/65123/) will not be updated anymore.
> 
> 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
> -----
> 
>   ocf-database-connector/README.md PRE-CREATION 
>   ocf-database-connector/pom.xml PRE-CREATION 
>   ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/connectors/OCFDatabaseConnection.java PRE-CREATION 
>   ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/connectors/OCFDatabaseConnector.java PRE-CREATION 
>   ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/connectors/OCFDatabaseConnectorProviderBase.java PRE-CREATION 
>   ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/connectors/gaian/GaianOCFConnector.java PRE-CREATION 
>   ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/connectors/gaian/GaianOCFConnectorProvider.java PRE-CREATION 
>   ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/ffdc/DatabaseConnectCheckedException.java PRE-CREATION 
>   ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/ffdc/OCFDatabaseCheckedExceptionBase.java PRE-CREATION 
>   ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/ffdc/OCFDatabaseConnectorErrorCode.java PRE-CREATION 
>   ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/ffdc/OCFDatabaseConnectorRuntimeException.java PRE-CREATION 
>   ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/util/PropertiesHelper.java PRE-CREATION 
>   ocf-database-connector/src/main/resources/gaian.properties PRE-CREATION 
>   ocf-database-connector/src/test/java/gaianocfconnector/GaianOCFConnectorTest.java PRE-CREATION 
>   ocf-database-connector/src/test/java/gaianocfconnector/UseGaianOCFConnector.java PRE-CREATION 
>   pom.xml ee2746a905f84359d426acfb1fa6d64c6310aaaf 
> 
> 
> Diff: https://reviews.apache.org/r/65435/diff/2/
> 
> 
> Testing
> -------
> 
> see test folder.
> Gaian has to set up in advance
> 
> 
> File Attachments
> ----------------
> 
> 0005-ATLAS-2298-05-Feb-code-review.patch
>   https://reviews.apache.org/media/uploaded/files/2018/02/05/292c1917-9deb-4ced-8bd6-1689fc45fbd4__0005-ATLAS-2298-05-Feb-code-review.patch
> 
> 
> Thanks,
> 
> Yao Li
> 
>


Re: Review Request 65435: ATLAS-2298 - Review of OCF Database Connector_New

Posted by David Radley <da...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65435/#review199965
-----------------------------------------------------------




ocf-database-connector/README.md
Lines 40 (patched)
<https://reviews.apache.org/r/65435/#comment280469>

    bad eol character



ocf-database-connector/README.md
Lines 42 (patched)
<https://reviews.apache.org/r/65435/#comment280472>

    typo sequene
    
    the sentence mentions we and you - I suggest using one or the other.



ocf-database-connector/README.md
Lines 58 (patched)
<https://reviews.apache.org/r/65435/#comment280470>

    bad eol character



ocf-database-connector/README.md
Lines 61 (patched)
<https://reviews.apache.org/r/65435/#comment280474>

    I thought impersonation would require a patch on top of Gaian. If this is the case we need to detail where to get this patch for impersonation to be able to be used here.



ocf-database-connector/README.md
Lines 69 (patched)
<https://reviews.apache.org/r/65435/#comment280471>

    same



ocf-database-connector/README.md
Lines 71 (patched)
<https://reviews.apache.org/r/65435/#comment280473>

    'here' is an extra word
    Typo Gian.
    We have not mention LDAP, I think we need to be careful mentioning products that might not be installed.
    We should not mention default passwords here - as in production this would not make sense.



ocf-database-connector/README.md
Lines 79 (patched)
<https://reviews.apache.org/r/65435/#comment280475>

    This text is assuming that people have access to a Gaian Ranger plugin. This is not currently available.
    
    You say "it is better to use **table function**. My understanding is that using the Ranger plugin requires the table function to be used. 
    
    You say "You need to change cst to your own table name". What is cst?



ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/connectors/gaian/GaianOCFConnector.java
Lines 35 (patched)
<https://reviews.apache.org/r/65435/#comment280477>

    incomplete sentence



ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/connectors/gaian/GaianOCFConnector.java
Lines 159 (patched)
<https://reviews.apache.org/r/65435/#comment280478>

    finally is driven for errors as well - so this debug message is not correct. I suggst moving this debug to the end of the try section.



ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/connectors/gaian/GaianOCFConnector.java
Lines 212 (patched)
<https://reviews.apache.org/r/65435/#comment280479>

    should we return this for an error?



ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/connectors/gaian/GaianOCFConnector.java
Lines 232 (patched)
<https://reviews.apache.org/r/65435/#comment280480>

    this if and body is a repeat of an previous if.



ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/ffdc/OCFDatabaseConnectorErrorCode.java
Lines 76 (patched)
<https://reviews.apache.org/r/65435/#comment280481>

    what is the user's valid information and valid use of OCF Connectors?



ocf-database-connector/src/main/resources/gaian.properties
Lines 6 (patched)
<https://reviews.apache.org/r/65435/#comment280482>

    It does not look right to include default passwords here, without talking about poc / dev scenarios and not production scenarios



ocf-database-connector/src/test/java/gaianocfconnector/GaianOCFConnectorTest.java
Lines 56 (patched)
<https://reviews.apache.org/r/65435/#comment280483>

    It looks like this class might a be unit test, but it seems that it requires Gaian to be running. If this cannot be run as a unit test - I suggest including instructions on how to run the tests in this file.



ocf-database-connector/src/test/java/gaianocfconnector/UseGaianOCFConnector.java
Lines 33 (patched)
<https://reviews.apache.org/r/65435/#comment280484>

    I suggest the test throws the excpetions rather than gobbles them. In this way the test fails if there is an error


- David Radley


On March 22, 2018, 11:23 a.m., Yao Li wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65435/
> -----------------------------------------------------------
> 
> (Updated March 22, 2018, 11:23 a.m.)
> 
> 
> Review request for atlas and Mandy Chessell.
> 
> 
> Repository: atlas
> 
> 
> Description
> -------
> 
> This is the new review request for ATLAS-2298 OCF Database Connector. The old review on [https://reviews.apache.org/r/65123/](https://reviews.apache.org/r/65123/) will not be updated anymore.
> 
> 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
> -----
> 
>   ocf-database-connector/README.md PRE-CREATION 
>   ocf-database-connector/pom.xml PRE-CREATION 
>   ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/connectors/OCFDatabaseConnection.java PRE-CREATION 
>   ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/connectors/OCFDatabaseConnector.java PRE-CREATION 
>   ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/connectors/OCFDatabaseConnectorProviderBase.java PRE-CREATION 
>   ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/connectors/gaian/GaianOCFConnector.java PRE-CREATION 
>   ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/connectors/gaian/GaianOCFConnectorProvider.java PRE-CREATION 
>   ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/ffdc/DatabaseConnectCheckedException.java PRE-CREATION 
>   ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/ffdc/OCFDatabaseCheckedExceptionBase.java PRE-CREATION 
>   ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/ffdc/OCFDatabaseConnectorErrorCode.java PRE-CREATION 
>   ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/ffdc/OCFDatabaseConnectorRuntimeException.java PRE-CREATION 
>   ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/util/PropertiesHelper.java PRE-CREATION 
>   ocf-database-connector/src/main/resources/gaian.properties PRE-CREATION 
>   ocf-database-connector/src/test/java/gaianocfconnector/GaianOCFConnectorTest.java PRE-CREATION 
>   ocf-database-connector/src/test/java/gaianocfconnector/UseGaianOCFConnector.java PRE-CREATION 
>   pom.xml ee2746a905f84359d426acfb1fa6d64c6310aaaf 
> 
> 
> Diff: https://reviews.apache.org/r/65435/diff/2/
> 
> 
> Testing
> -------
> 
> see test folder.
> Gaian has to set up in advance
> 
> 
> File Attachments
> ----------------
> 
> 0005-ATLAS-2298-05-Feb-code-review.patch
>   https://reviews.apache.org/media/uploaded/files/2018/02/05/292c1917-9deb-4ced-8bd6-1689fc45fbd4__0005-ATLAS-2298-05-Feb-code-review.patch
> 
> 
> Thanks,
> 
> Yao Li
> 
>


Re: Review Request 65435: ATLAS-2298 - Review of OCF Database Connector_New

Posted by Yao Li <be...@126.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65435/
-----------------------------------------------------------

(Updated March 22, 2018, 11:23 a.m.)


Review request for atlas and Mandy Chessell.


Changes
-------

update OCF Database connector based on the reviews


Repository: atlas


Description
-------

This is the new review request for ATLAS-2298 OCF Database Connector. The old review on [https://reviews.apache.org/r/65123/](https://reviews.apache.org/r/65123/) will not be updated anymore.

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 (updated)
-----

  ocf-database-connector/README.md PRE-CREATION 
  ocf-database-connector/pom.xml PRE-CREATION 
  ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/connectors/OCFDatabaseConnection.java PRE-CREATION 
  ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/connectors/OCFDatabaseConnector.java PRE-CREATION 
  ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/connectors/OCFDatabaseConnectorProviderBase.java PRE-CREATION 
  ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/connectors/gaian/GaianOCFConnector.java PRE-CREATION 
  ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/connectors/gaian/GaianOCFConnectorProvider.java PRE-CREATION 
  ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/ffdc/DatabaseConnectCheckedException.java PRE-CREATION 
  ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/ffdc/OCFDatabaseCheckedExceptionBase.java PRE-CREATION 
  ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/ffdc/OCFDatabaseConnectorErrorCode.java PRE-CREATION 
  ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/ffdc/OCFDatabaseConnectorRuntimeException.java PRE-CREATION 
  ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/util/PropertiesHelper.java PRE-CREATION 
  ocf-database-connector/src/main/resources/gaian.properties PRE-CREATION 
  ocf-database-connector/src/test/java/gaianocfconnector/GaianOCFConnectorTest.java PRE-CREATION 
  ocf-database-connector/src/test/java/gaianocfconnector/UseGaianOCFConnector.java PRE-CREATION 
  pom.xml ee2746a905f84359d426acfb1fa6d64c6310aaaf 


Diff: https://reviews.apache.org/r/65435/diff/2/

Changes: https://reviews.apache.org/r/65435/diff/1-2/


Testing
-------

see test folder.
Gaian has to set up in advance


File Attachments
----------------

0005-ATLAS-2298-05-Feb-code-review.patch
  https://reviews.apache.org/media/uploaded/files/2018/02/05/292c1917-9deb-4ced-8bd6-1689fc45fbd4__0005-ATLAS-2298-05-Feb-code-review.patch


Thanks,

Yao Li


Re: Review Request 65435: ATLAS-2298 - Review of OCF Database Connector_New

Posted by Yao Li <be...@126.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65435/
-----------------------------------------------------------

(Updated Feb. 5, 2018, 2:26 p.m.)


Review request for atlas and Mandy Chessell.


Changes
-------

update the reame.md


Repository: atlas


Description
-------

This is the new review request for ATLAS-2298 OCF Database Connector. The old review on [https://reviews.apache.org/r/65123/](https://reviews.apache.org/r/65123/) will not be updated anymore.

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

  ocf-database-connector/README.md PRE-CREATION 
  ocf-database-connector/pom.xml PRE-CREATION 
  ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/connectors/OCFDatabaseConnection.java PRE-CREATION 
  ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/connectors/OCFDatabaseConnector.java PRE-CREATION 
  ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/connectors/OCFDatabaseConnectorProviderBase.java PRE-CREATION 
  ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/connectors/OCFDatabaseDataManager.java PRE-CREATION 
  ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/connectors/gaian/GaianOCFConnector.java PRE-CREATION 
  ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/connectors/gaian/GaianOCFConnectorProvider.java PRE-CREATION 
  ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/ffdc/DatabaseAccessCheckedException.java PRE-CREATION 
  ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/ffdc/OCFDatabaseCheckedExceptionBase.java PRE-CREATION 
  ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/ffdc/OCFDatabaseConnectorErrorCode.java PRE-CREATION 
  ocf-database-connector/src/main/java/org/apache/atlas/ocfdbconnector/ffdc/OCFDatabaseConnectorRuntimeException.java PRE-CREATION 
  pom.xml c15e0dad 


Diff: https://reviews.apache.org/r/65435/diff/1/


Testing
-------

see test folder.
Gaian has to set up in advance


File Attachments (updated)
----------------

0005-ATLAS-2298-05-Feb-code-review.patch
  https://reviews.apache.org/media/uploaded/files/2018/02/05/292c1917-9deb-4ced-8bd6-1689fc45fbd4__0005-ATLAS-2298-05-Feb-code-review.patch


Thanks,

Yao Li