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

Set up publishing to Entrepot #22

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

guilgaly
Copy link
Contributor

@guilgaly guilgaly commented Nov 29, 2018

Pull Request Checklist

  • Have you read through the contributor guide?
  • Have you added tests for any changed functionality?

Purpose

Set up publishing to Entrepot

Background Context

N/A

References

Depends on pull request #19

.travis.yml Outdated Show resolved Hide resolved
project/Publish.scala Outdated Show resolved Hide resolved
project/Publish.scala Outdated Show resolved Hide resolved
@guilgaly guilgaly force-pushed the publish-to-zen-entrepot branch from edf0377 to dd31821 Compare November 30, 2018 10:06
@guilgaly
Copy link
Contributor Author

Looks like we have a "random" error in a test... I had a test failure in WithPlayTransactionSpec (in the last test, "not insert an author or the book when the book table is written with a typo") with the following stacktrace, but it disappeared when I restarted the CI job.

[error]   ! fail when inserting into a non existent table
[error]    java.lang.IllegalArgumentException: No matching handler: 7f625d67-46d2-4250-9e8c-54d3eeed289d (Driver.java:80)
[error] acolyte.jdbc.Driver.connect(Driver.java:80)
[error] acolyte.jdbc.Driver.connect(Driver.java:18)
[error] acolyte.jdbc.play.AcolyteDatabase.getConnection(AcolyteDatabase.scala:64)
[error] com.zengularity.querymonad.module.playsql.database.WithPlayTransaction.connection$lzycompute$1(WithPlayTransaction.scala:19)
[error] com.zengularity.querymonad.module.playsql.database.WithPlayTransaction.com$zengularity$querymonad$module$playsql$database$WithPlayTransaction$$connection$1(WithPlayTransaction.scala:19)
[error] com.zengularity.querymonad.module.playsql.database.WithPlayTransaction.apply(WithPlayTransaction.scala:20)
[error] com.zengularity.querymonad.core.database.LowPriorityCompose$$anon$2.apply(ComposeWithCompletion.scala:42)
[error] com.zengularity.querymonad.core.database.QueryRunner$DefaultRunner.apply(QueryRunner.scala:21)
[error] com.zengularity.querymonad.test.module.playsql.database.WithPlayTransactionSpec.$anonfun$new$33(WithPlayTransactionSpec.scala:67)
[error] com.zengularity.querymonad.test.module.playsql.database.WithPlayTransactionSpec.$anonfun$new$32(WithPlayTransactionSpec.scala:69)
[error] com.zengularity.querymonad.test.module.playsql.database.WithPlayTransactionSpec.$anonfun$new$29(WithPlayTransactionSpec.scala:69)
[error] com.zengularity.querymonad.test.module.playsql.utils.WithPlayTransactionUseCases$.testRunner(WithPlayTransactionUseCases.scala:20)
[error] com.zengularity.querymonad.test.module.playsql.utils.WithPlayTransactionUseCases$.$anonfun$test1$2(WithPlayTransactionUseCases.scala:24)
[error] com.zengularity.querymonad.test.module.playsql.database.WithPlayTransactionSpec.$anonfun$new$28(WithPlayTransactionSpec.scala:60)

I'll try to see if I can find something...

@guilgaly
Copy link
Contributor Author

Looks like the above error is caused by a thread-safety issue in Accolyte ; I can reproduce the error semi-randomly when I execute the tests as-is, but I can no longer reproduce it if I synchronize the test database creation (new AcolyteDatabase(...)). Anyway, it's unrelated to this PR.

@cchantep
Copy link
Contributor

I'm surprise, Acolyte used fully isolated connection, I'm would rather check the connection handler it's given, as that could be concurrent in tests (and if there is mutations there ...)

@guilgaly guilgaly force-pushed the publish-to-zen-entrepot branch from b33a113 to bb08320 Compare November 30, 2018 14:52
@guilgaly guilgaly force-pushed the publish-to-zen-entrepot branch from bb08320 to 359cc3f Compare January 21, 2019 15:39
@guilgaly
Copy link
Contributor Author

I'd like to finally finish those old PRs... I think this one should be good to merge if there are no more problems with it ? @stankoua @cchantep

(The random test failure is fixed by cchantep/acolyte#93 but we'll need a new release of Acolyte to benefit from it. Anyway, not specific to this PR.)

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

Successfully merging this pull request may close these issues.

2 participants