Support for Oracle ETL DataEndpoint (readonly) #34

smgallo · 2017-01-20T18:48:28Z

Implement an ETL DataEndpoint for reading from an Oracle database.

Description

Implements support for importing data from an Oracle database. In addition to adding an Oracle DataEndpoint, the following changes were made as a result of necessary features for importing

More flexible database parameters in .ini file (not all databases require all parameters). For example, Oracle supports Easy Connect and Local database connection methods and a port does not always need to be specified.
Include support for specifying ORDER BY in ETL queries
The PDOOCI wrapper for OCI8 is included. The existing PDO code included with PHP is no longer supported.
Minor code cleanup and style fixes
Improve error messages
Removes the old Sybase code

Note that Oracle requires downloading the Oracle Instant Client libraries.

Motivation and Context

We will need to pull organizational hierarchy and user affiliations from UB's data warehouse (Infosource). This will also assist SDSC with migration from their existing Oracle system.

Tests performed

Imported the organizational hierarchy from UB's Oracle instance.

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

My code follows the code style of this project as found in the CONTRIBUTING document.
I have added tests to cover my changes.
All new and existing tests passed.

- More flexible database parameters in .ini file (not all databases require all parameters) - Improve handling of DB Engine subclasses (e.g., PostgresDB, MySQLDB) - Improved parameter validation on a per-engine basis - Added methods to iDatabase that should have been there before - General code cleanup - Improve comments

…aendpoint

… or table exists

jsperhac · 2017-01-20T19:27:43Z

classes/ETL/DataEndpoint/aRdbmsEndpoint.php

+            $this->handle = DB::factory($this->config);
+        } catch (Exception $e) {
+            $msg = "Error connecting to data endpoint '" . $this->name . "'. " . $e->getMessage();
+             $this->logAndThrowException($msg);


Extra whitespace on line 136?

Interesting that phpcs didn't complain about that.

jsperhac · 2017-01-20T20:00:18Z

classes/ETL/DataEndpoint/Postgres.php

    {
-        if ( empty($schemaName) ) {
+        if ( null === $schemaName ) {
+            $schemaName = $this->getSchema();


Lines 44-49 (from Postgres.php) are seen frequently in these classes that extend aRdbmsEndpoint--is it worth putting a function in aRdbmsEndpoint to handle the test for SchemaName and the logging action?

Yes, that makes sense.

jsperhac · 2017-01-20T20:37:05Z

classes/ETL/DataEndpoint/Oracle.php

+        // See http://www.postgresql.org/docs/current/static/catalogs.html
+
+        $sql = "SELECT
+username AS name


Formatting seems off for these SELECT strings (lines 54, 114, 160).

My editor doesn't indent bare strings on the line. Plus, the extra indentation makes it more difficult to read in the debug output.

jsperhac · 2017-01-20T20:39:35Z

classes/ETL/DataEndpoint/README_ORACLE.md

@@ -0,0 +1,219 @@
+# Oracle Data Endpoint


Does it make more sense to put this file under configuration/ or docs/, perhaps?

Overall, our documentation needs to be updated to reflect Oracle support (and emphasize that it is read-only for ETL), as well.

I moved it to docs/etl/data-endpoint-oracle.md. I also need to update the ETL documentation overall and move it out of confluence. There are a lot of docs to move.

plessbd

Unless the updates to dependencies are directly related to the new dependency I would want to move those updates to a different pull request

plessbd

Fix travis CI build failures:

Errors:
https://travis-ci.org/ubccr/xdmod/jobs/193879363#L320
https://travis-ci.org/ubccr/xdmod/jobs/193879363#L520

Warnings (decide if needs to be fixed or not):
https://travis-ci.org/ubccr/xdmod/jobs/193879363#L333

Unit test failures:
https://travis-ci.org/ubccr/xdmod/jobs/193879364#L324

plessbd · 2017-01-21T07:06:18Z

tools/etl/etl_overseer.php

@@ -689,8 +728,8 @@ function usage_and_exit($msg = null)

    NOTE: Date and time options support "+1 day", "now", "now - 1 day", etc. notation.

-EOMSG
-        );
+        EOMSG;


heredoc requires closing tag to be at the beginning of the line.

Warning

It is very important to note that the line with the closing identifier must contain no other characters, except a semicolon (;). That means especially that the identifier may not be indented, and there may not be any spaces or tabs before or after the semicolon. It's also important to realize that the first character before the closing identifier must be a newline as defined by the local operating system. This is \n on UNIX systems, including Mac OS X. The closing delimiter must also be followed by a newline.

If this rule is broken and the closing identifier is not "clean", it will not be considered a closing identifier, and PHP will continue looking for one. If a proper closing identifier is not found before the end of the current file, a parse error will result at the last line.

I believe this is what is now causing travis to fail with

PHP Parse error: syntax error, unexpected $end in tools/etl/etl_overseer.php on line 737

…aendpoint Conflicts: tools/etl/etl_overseer.php

…to etl/oracle-dataendpoint Conflicts: tools/etl/etl_overseer.php

smgallo · 2017-01-21T17:50:22Z

OK, all style and unit test issues are fixed as well as changes requested by @jsperhac and @plessbd

plessbd · 2017-01-22T14:27:35Z

Looks good to me once the composer.lock is fixed to not update dependencies.

smgallo · 2017-01-22T20:21:13Z

Composer lock file reverted. The new file only has timestamps updated and the requirements for PDOOCI. I needed to clear my composer cache or else running composer require would update the packages to the new versions.

* Prepare for adding Oracle support - More flexible database parameters in .ini file (not all databases require all parameters) - Improve handling of DB Engine subclasses (e.g., PostgresDB, MySQLDB) - Improved parameter validation on a per-engine basis - Added methods to iDatabase that should have been there before - General code cleanup - Improve comments * Allow methods to override default schema name when checking if schema or table exists * Add support for ORDER BY in ETL queries * Improve debug readability and error messages * Add modern PDO support for Oracle (PDOOCI wraps the OCI8 library) * Support for Oracle ETL data endpoint (readonly) * Style fixes as per phpcs * Address style and unit test errors, rmove underscores from private class members

smgallo added 9 commits January 13, 2017 13:50

Improve rdbms dataendpoint error messages

133111a

Merge branch 'xdmod6.6' of github.com:ubccr/xdmod into etl/oracle-dat…

41b3660

…aendpoint

Allow methods to override default schema name when checking if schema…

e44857c

… or table exists

Add support for ORDER BY in ETL queries

ba6862e

Improve debug readability

6d77b52

Add modern PDO support for Oracle (PDOOCI wraps the OCI8 library)

7b32142

Support for Oracle ETL data endpoint (readonly)

c6cef7e

Style fixes as per phpcs

999acea

smgallo added the enhancement Enhancement of the functionality of an existing feature label Jan 20, 2017

smgallo added this to the v6.6.0 milestone Jan 20, 2017

smgallo requested review from plessbd, jsperhac and tyearke January 20, 2017 18:48

jsperhac reviewed Jan 20, 2017

View reviewed changes

Addressed comments from @jsperhac

9c49f02

plessbd suggested changes Jan 20, 2017

View reviewed changes

smgallo and others added 2 commits January 20, 2017 19:55

Address style and unit test errors

0b08489

Merge branch 'xdmod6.6' into etl/oracle-dataendpoint

d53eb71

plessbd reviewed Jan 21, 2017

View reviewed changes

smgallo added 6 commits January 21, 2017 09:49

TravisCI build fixes

d603ce2

Merge branch 'xdmod6.6' of github.com:ubccr/xdmod into etl/oracle-dat…

7630e6e

…aendpoint Conflicts: tools/etl/etl_overseer.php

More style fixes

c0e48e3

Merge branch 'etl/oracle-dataendpoint' of github.com:smgallo/xdmod in…

2ee675f

…to etl/oracle-dataendpoint Conflicts: tools/etl/etl_overseer.php

Remove __construct() from iDatabase, clean up code style

57a3c08

Remove underscores from private class members

2ea92dc

Revert composer updates

3cc0fc4

plessbd approved these changes Jan 22, 2017

View reviewed changes

smgallo merged commit feddfeb into ubccr:xdmod6.6 Jan 23, 2017

smgallo deleted the etl/oracle-dataendpoint branch January 23, 2017 14:05

tyearke mentioned this pull request Jan 23, 2017

Added license info for PDOOCI #37

Merged

2 tasks

smgallo added the Category:ETL Extract Transform Load label Apr 26, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support for Oracle ETL DataEndpoint (readonly) #34

Support for Oracle ETL DataEndpoint (readonly) #34

smgallo commented Jan 20, 2017

jsperhac Jan 20, 2017

smgallo Jan 20, 2017

jsperhac Jan 20, 2017

smgallo Jan 20, 2017

jsperhac Jan 20, 2017

smgallo Jan 20, 2017

jsperhac Jan 20, 2017

jsperhac Jan 20, 2017

smgallo Jan 20, 2017

plessbd left a comment

plessbd left a comment

plessbd Jan 21, 2017

smgallo commented Jan 21, 2017

plessbd commented Jan 22, 2017

smgallo commented Jan 22, 2017

Support for Oracle ETL DataEndpoint (readonly) #34

Support for Oracle ETL DataEndpoint (readonly) #34

Conversation

smgallo commented Jan 20, 2017

Description

Motivation and Context

Tests performed

Types of changes

Checklist:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

plessbd left a comment

Choose a reason for hiding this comment

plessbd left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

smgallo commented Jan 21, 2017

plessbd commented Jan 22, 2017

smgallo commented Jan 22, 2017