search

pietermartin / sqlg

TinkerPop graph over sql
MIT License
246 stars 51 forks source link

Upgrades Tinkerpop to 3.4.1 and SLF4J to 1.7.25, adds Travis/docker-compose scripting #358

Closed ssalevan closed 5 years ago

ssalevan commented 5 years ago

This PR upgrades both TInkerpop to version 3.4.1 and SLF4J to version 1.7.25 to enable compatibility with sqlg consumer libraries, such as gremlin-scala. The Tinkerpop upgrade introduced numerous API refactors which I've adapted here, most of them largely no-ops to ensure conformance.

To test that these adaptations haven't introduced any weird bugs into the codebase, I've reconfigured the Travis CI buildfile and Docker build scripting to support the Postgres defaults encoded within the sqlg test suite. The Postgres dialect-facing tests ran cleanly, as did all other non-DB specific tests, but several targets such as the MySQL and MS SQL Server integration suites did not, as we have not yet set up databases for these tests.

With the introduction of a docker-compose file, included here, we should be able to add support for all remaining databases and integrate full functional/integration tests for branch and master integrations. As these machines run with 2 cores, we should be able to have everything run under the 50 minute limit with 4 Maven threads. I'd be happy to take this on in a future review as we continue our sqlg integration over here.

Let me know what you think and thanks for the consideration!

pietermartin commented 5 years ago

Great, tx for the PR, I'll test the other db's this evening and merge the PR if all is well.

pietermartin commented 5 years ago

Thanks for the PR.

pietermartin commented 5 years ago

I don't see any citus configuration in the docker scripts. Did you exclude them from the postgres tests? In particular TestSharding and TestShardingGremlin are citusdb tests.

ssalevan commented 5 years ago

I was not aware that Citus-based systems were supported and have not excluded any tests, but I'd be glad to add Citus support to the Docker configuration. It looks like the tests expect a Postgres-like database to be listening on both port 9700 and port 5432; is one of these where a Citus-style database should be listening?

pietermartin commented 5 years ago

The setup I use is,

export PATH=$PATH:/usr/lib/postgresql/10/bin
pg_ctl -D citus/coordinator -o "-p 9700" -l coordinator_logfile start
pg_ctl -D citus/worker1 -o "-p 9701" -l worker1_logfile restart
pg_ctl -D citus/worker2 -o "-p 9702" -l worker2_logfile restart
pg_ctl -D citus/worker3 -o "-p 9703" -l worker3_logfile restart
pg_ctl -D citus/worker4 -o "-p 9704" -l worker4_logfile restart

All 4 instances are standard postgres databases with the citus extensions installed. The postgres tests point to 9700.