You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@pekko.apache.org by "pjfanning (via GitHub)" <gi...@apache.org> on 2023/08/14 12:15:44 UTC
[GitHub] [incubator-pekko-connectors] pjfanning opened a new pull request, #223: scala3 support for slick
pjfanning opened a new pull request, #223:
URL: https://github.com/apache/incubator-pekko-connectors/pull/223
relates to #151
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] scala3 support for slick [incubator-pekko-connectors]
Posted by "mdedetrich (via GitHub)" <gi...@apache.org>.
mdedetrich commented on PR #223:
URL: https://github.com/apache/incubator-pekko-connectors/pull/223#issuecomment-1958997035
> Alternatively, would it be possible to set up our GitHub Actions so that we can publish a snapshot built off a branch/PR?
I wanted to do this at some point I just never got around to it. Note that nothing is stopping us from publishing a snapshot locally from your machine, its just that you have to be careful about the version name that you set as it can break https://github.com/pjfanning/sbt-pekko-build/blob/main/src/main/scala/com/github/pjfanning/pekkobuild/PekkoDependency.scala
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] scala3 support for slick [incubator-pekko-connectors]
Posted by "mdedetrich (via GitHub)" <gi...@apache.org>.
mdedetrich commented on PR #223:
URL: https://github.com/apache/incubator-pekko-connectors/pull/223#issuecomment-1925575038
@pjfanning I just cleaned up the branch which included some rebases so I would just pull the latest changes ignoring the local ones.
Good news is that Slick 3.5.0-M4 seems to work fine with none of the previous problems that we were experiencing.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] scala3 support for slick [incubator-pekko-connectors]
Posted by "mdedetrich (via GitHub)" <gi...@apache.org>.
mdedetrich commented on PR #223:
URL: https://github.com/apache/incubator-pekko-connectors/pull/223#issuecomment-1959051742
Replied at https://github.com/slick/slick/discussions/2891#discussioncomment-8554394
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] scala3 support for slick [incubator-pekko-connectors]
Posted by "pjfanning (via GitHub)" <gi...@apache.org>.
pjfanning commented on code in PR #223:
URL: https://github.com/apache/incubator-pekko-connectors/pull/223#discussion_r1516167497
##########
slick/src/main/scala/org/apache/pekko/stream/connectors/slick/javadsl/Slick.scala:
##########
@@ -54,6 +51,7 @@ object Slick {
session: SlickSession,
query: String,
mapper: JFunction[SlickRow, T]): Source[T, NotUsed] = {
+
Review Comment:
just spreading out the code to make it more readable
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] scala3 support for slick [incubator-pekko-connectors]
Posted by "mdedetrich (via GitHub)" <gi...@apache.org>.
mdedetrich commented on PR #223:
URL: https://github.com/apache/incubator-pekko-connectors/pull/223#issuecomment-1958814213
Please don't merge this PR until we make a discussion about whether its appropriate for us as a snapshot
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] scala3 support for slick [incubator-pekko-connectors]
Posted by "pjfanning (via GitHub)" <gi...@apache.org>.
pjfanning commented on PR #223:
URL: https://github.com/apache/incubator-pekko-connectors/pull/223#issuecomment-1958987553
I agree that it is too time consuming for us to do releases to test non-ASF jars. If someone really wanted to test this, they could build pekko-connectors-slick jars themselves by doing a git checkout of this PR's branch and running sbt commands.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] scala3 support for slick [incubator-pekko-connectors]
Posted by "mdedetrich (via GitHub)" <gi...@apache.org>.
mdedetrich commented on code in PR #223:
URL: https://github.com/apache/incubator-pekko-connectors/pull/223#discussion_r1516110691
##########
slick/src/test/java/docs/javadsl/SlickTest.java:
##########
Review Comment:
Can you try reverting the change in this file from `10` seconds to `5` seconds? This was done in diagnosing an error with flaky tests on an earlier Slick version however it was unrelated (and latest slick version no longer has this flakiness)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] scala3 support for slick [incubator-pekko-connectors]
Posted by "mdedetrich (via GitHub)" <gi...@apache.org>.
mdedetrich commented on PR #223:
URL: https://github.com/apache/incubator-pekko-connectors/pull/223#issuecomment-1822232666
@pjfanning There are MiMa related failures, although tbh we never agreed for MiMa on this pekko module and Slick 3.5 is slated for 1.1.x anyways
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
[GitHub] [incubator-pekko-connectors] mdedetrich commented on pull request #223: scala3 support for slick
Posted by "mdedetrich (via GitHub)" <gi...@apache.org>.
mdedetrich commented on PR #223:
URL: https://github.com/apache/incubator-pekko-connectors/pull/223#issuecomment-1692537114
> The Slick tests haven't been all that flaky up until now.
Yes this is my recollection as well, the flakiness is only occurring with Slick Scala 3
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
[GitHub] [incubator-pekko-connectors] mdedetrich commented on pull request #223: scala3 support for slick
Posted by "mdedetrich (via GitHub)" <gi...@apache.org>.
mdedetrich commented on PR #223:
URL: https://github.com/apache/incubator-pekko-connectors/pull/223#issuecomment-1690681030
So the slick tests are definitely flaky, I added a commit which seems to have helped (i.e. only failing ~30% of the time). I don't recall if the Slick tests were normally this flaky (don't remember this being the case) and it only seems to be happening on Scala 3.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] scala3 support for slick [incubator-pekko-connectors]
Posted by "mdedetrich (via GitHub)" <gi...@apache.org>.
mdedetrich commented on PR #223:
URL: https://github.com/apache/incubator-pekko-connectors/pull/223#issuecomment-1958782718
So it is being [asked](https://github.com/slick/slick/discussions/2891#discussioncomment-8550269) if we can publish these changes so that people can test Slick 3.5.0-RC1 before Slick does a full release. Since 1.0.x branch is already setup if we want to do snapshots we can just merge this PR as is, milestone would obviously require a proper vote which I am not against and given what we discussed at https://github.com/apache/incubator-pekko-connectors/discussions/443#discussioncomment-8506668 creating a milestone right now (aside from just Slick 3.5.0-RC1) may not be a bad idea since we already did quite a few major dependency updates.
@pjfanning wdyt?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] scala3 support for slick [incubator-pekko-connectors]
Posted by "raboof (via GitHub)" <gi...@apache.org>.
raboof commented on PR #223:
URL: https://github.com/apache/incubator-pekko-connectors/pull/223#issuecomment-1958975082
Ideally I think releases should not depend on snapshot/RC dependencies, and the main branch should always be in a release-able state. I guess we can deviate from that if there is a strong reason for it. Alternatively, would it be possible to set up our GitHub Actions so that we can publish a snapshot built off a branch/PR?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
[GitHub] [incubator-pekko-connectors] pjfanning commented on pull request #223: scala3 support for slick
Posted by "pjfanning (via GitHub)" <gi...@apache.org>.
pjfanning commented on PR #223:
URL: https://github.com/apache/incubator-pekko-connectors/pull/223#issuecomment-1690699159
The Slick tests haven't been all that flaky up until now.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] scala3 support for slick [incubator-pekko-connectors]
Posted by "pjfanning (via GitHub)" <gi...@apache.org>.
pjfanning commented on code in PR #223:
URL: https://github.com/apache/incubator-pekko-connectors/pull/223#discussion_r1516303694
##########
slick/src/test/java/docs/javadsl/SlickTest.java:
##########
Review Comment:
test timeouts reverted
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] scala3 support for slick [incubator-pekko-connectors]
Posted by "mdedetrich (via GitHub)" <gi...@apache.org>.
mdedetrich commented on code in PR #223:
URL: https://github.com/apache/incubator-pekko-connectors/pull/223#discussion_r1516109342
##########
slick/src/main/scala/org/apache/pekko/stream/connectors/slick/javadsl/Slick.scala:
##########
@@ -54,6 +51,7 @@ object Slick {
session: SlickSession,
query: String,
mapper: JFunction[SlickRow, T]): Source[T, NotUsed] = {
+
Review Comment:
nit: Random new line?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] scala3 support for slick [incubator-pekko-connectors]
Posted by "pjfanning (via GitHub)" <gi...@apache.org>.
pjfanning merged PR #223:
URL: https://github.com/apache/incubator-pekko-connectors/pull/223
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org