Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Conversation

ssalevan
Copy link
Contributor

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
Copy link
Owner

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

@pietermartin pietermartin merged commit ca6898f into pietermartin:master Aug 4, 2019
@pietermartin
Copy link
Owner

Thanks for the PR.

@pietermartin
Copy link
Owner

pietermartin commented Aug 4, 2019

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
Copy link
Contributor Author

ssalevan commented Aug 5, 2019

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
Copy link
Owner

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants