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