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

Doobie integration #541

Merged
merged 6 commits into from
Mar 4, 2019
Merged

Doobie integration #541

merged 6 commits into from
Mar 4, 2019

Conversation

asaushkin
Copy link
Contributor

These changes add doobie integration with Hikari connection pool. Supported databases are H2 and PostgreSQL. Currently Slick integration is not removed and may be used as well as early.

Default master configuration is H2 over new Doobie with Hikari coonection pool.

To avaid libraries conflict removed `cats` version 0.9 dependency,
because newer version 1.4 of `cats` is depended on doobie.

Commented code that interferes to Scala version upgrade.
This commit intended for implementation of the JobRepository
with Doobie. Now HikariJobRepository can work with
PostgreSQL as well as with H2 database.

Because of different DDL syntax migration directory for
the H2 directory was moved to /db/migrations/h2 and
PostgreSQL migration scripts were created into
/db/migrations/postgresql

New class JobDetailsRecord - helper class intended for data
extraction with Doobie.

HikariDataSourceTransactor is Doobie transactor with Hikari
DB pool.

HikariJobRepository - base implementation of data manipulation
with Doobie.

HikariRepoSpec - abstract tests for  HikariJobRepository.

HikariH2RepoSpec - concrete tests for H2

HikariPgRepoSpec - concrete tests for PostgreSQL. This class
marked as "abstract" to not break any CI builds.

You can remove abstract marker after test environment will be ready
for PostgreSQL.
@asaushkin asaushkin marked this pull request as ready for review March 2, 2019 14:21
HikariPgRepoSpec can't pass Travis CI tests. Mark it abstract
while Travis CI will be ready for it.
@dos65
Copy link
Contributor

dos65 commented Mar 4, 2019

Great! Thanks for your contribution. Now, we almost ready to publish a new release.

Everything is fine except tests and Travis setup.
Unfortunately, last commits about Travis don't fit for our case. It's hard to explain why, but we use two CI servers: Travis and Jenkins. Moving this spec to integrations tests is the best way to keep these tests. There we can run PostgreSQL as a docker container and this approach allows us not to care about an environment and run these test anywhere where docker is installed.

Actually, it would be enough just to remove last 5 commits to merge this PR. If you don't have enough time, I can complete this testing stuff after merge by myself.

@asaushkin
Copy link
Contributor Author

I think only last four commits are extra. This one 8dfd163 is mandatory for the project build. I'd appreciate you help if you will convert these tests to the integration test by yourself.

@dos65
Copy link
Contributor

dos65 commented Mar 4, 2019

Yes, you right - 4 would be enough.
Could you please edit history and remove these commits?

@asaushkin
Copy link
Contributor Author

I've just removed last three commits. The fourth mark Pg test as abtract and is mandatory to run tests without any fail.

@dos65 dos65 merged commit 556493c into Hydrospheredata:master Mar 4, 2019
@asaushkin asaushkin deleted the doobie branch March 4, 2019 13:08
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