Allow customization of the data source implementation

[jgustie]

In the process of upgrading to 2.x I noticed that the SqlgGraph.open method that accepted a customized data source was made private in acbd03e6faad3a8f6bf8f8000d4f94fd0bffad99, making it impossible to use custom implementations of SqlgDataSource (in my case, a Tomcat JDBC backed connection pool not registered with JNDI).

This PR improves SqlgDataSourceFactory by borrowing heavily from the TinkerPop GraphFactory class to allow customization of the SqlgDataSource implementation via configuration properties. This ensures only the single argument SqlgGraph.open methods are public while still allowing customization (hopefully this is now more clear).

I gave the property used to configure the data source a "sqlg." prefix (this is consistent with Graph.GRAPH which uses the "gremlin." prefix); I have found Configuration.subset to be very useful for isolating configuration parameters when they are namespaced like this.

It does not appear that the JDBC URL configuration property is still required by SqlgGraph, it may be worth removing that restriction in a future version (it is required by both the JNDI and C3P0 Sqlg data sources, but I was careful not to introduce the requirement to the SqlgDataSourceFactory).

[jgustie]

I just realized the property that used to allow configuration like this was the jdbc.factory configuration property, but that was eliminated in d18450e32c43fdbe8c098f002ae7ded4e9d0c72d along with the whole idea of a data source factory.

[pietermartin]

Looks good thanks. However the project does not compile. SqlgAbstractGraphProvider can not see SqlgDataSourceFactory which you gave package scope.

Also can we try writing a test for creating a graph with the custom data source? Maybe just instantiating the C3PODatasource?

[jgustie]

The SqlgAbstractGraphProvider was hiding behind an unreachable code error (I guess I'm the only one still using Eclipse these days) so I missed it.

I still cannot get the tests running locally (I feel like this happens to me every time), it seems I'm missing some database instances and the Travis build doesn't seem to be used anymore because the ports didn't match up. I tried to make the new test do the same thing that the other data source tests were doing, hopefully that works.

While I was writing the test, I realized that actually implementing a data source required duplicating SqlgPlugin loading logic found in the existing data source implementations: I moved that logic to static interface methods on SqlgPlugin, hopefully that is OK.

Finally, I snuck in an extra setting on the C3P0DataSource: it would be nice if all of the jdbc.* properties were propagated (reflectively?) to allow for some additional customization via the configuration (we use Configuration.subset all the time). The setting I exposed is the number of connection attempts (in test environments we set that from 30 down to 1 so the tests do not waste a bunch of time trying to connect to a non-existent database).

[pietermartin]

Did you manage to get the tests running? You don't really need to run the TinkerPop tests in which case only sqlgraphdb is needed. For TinkerPop's tests there is a tp3.databases file which contains all the databases that it needs.

The primary pain is that the tests by default run on port 9000, 9001, 9002, 9003, 9004 I moved it from the default port because of CitusDB and not to interfere with my day job's settings.

Reckon I need to remove CitusDB tests from the default settings as most people won't be using CitusDB.

I have given up on using Travis and run TeamCity locally from home at http://www.sqlg.org:8111 Travis was simply wasting to much of my time trying to get it to work with all the databases and memory requirements.

[jgustie]

I was able to get them to run, I also needed port 9700 (I'm running Postgres in Docker so I just use -p 9000:5432 -p 9001:5432 -p 9002:5432 -p 9003:5432 -p 9004:5432 -p 9700:5432 to map all the ports when I call docker run).

I hope they weren't related to these changes, but I did see a bunch (20+) of TestTopologyUpgrade errors/failures (from "gui_schema" does not exist to "V_person" already exists). There were also a lot of org.umlg.sqlg.test.topology.* errors around not being able to drop the role "sqlgReadOnly" because of table dependencies ("VIKINGS" and "EVENT"?).

[pietermartin]

Ah, yes I'd have to look again to remember what exactly the setup is for the read only users. They pass by me so no need to worry about it. Thanks