You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@pekko.apache.org by "He-Pin (via GitHub)" <gi...@apache.org> on 2024/03/22 12:11:59 UTC
[PR] chore: Add MonoToFuture to reduce convertion. [pekko-persistence-r2dbc]
He-Pin opened a new pull request, #96:
URL: https://github.com/apache/pekko-persistence-r2dbc/pull/96
Motivation:
`toFuture.asScala` seems quite boring, we can convert it to Scala Future directly.
Result:
Less allocation/conversion
--
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] chore: Add MonoToFuture to reduce conversion. [pekko-persistence-r2dbc]
Posted by "laglangyue (via GitHub)" <gi...@apache.org>.
laglangyue commented on code in PR #96:
URL: https://github.com/apache/pekko-persistence-r2dbc/pull/96#discussion_r1535638761
##########
core/src/test/scala/org/apache/pekko/persistence/r2dbc/internal/MonoToFutureSpec.scala:
##########
@@ -0,0 +1,27 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * license agreements; and to You under the Apache License, version 2.0:
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * This file is part of the Apache Pekko project, which was derived from Akka.
+ */
+
+package org.apache.pekko.persistence.r2dbc.internal
+
+import org.scalatest.TestSuite
+import org.scalatest.concurrent.ScalaFutures
+import org.scalatest.matchers.should.Matchers
+import org.scalatest.wordspec.AnyWordSpec
+import reactor.core.publisher.Mono
+
+class MonoToFutureSpec extends AnyWordSpec with ScalaFutures with TestSuite with Matchers {
+ "MonoToFutureSpec" should {
+ "convert a Mono to a Future" in {
+ val r = Mono.just("pekko")
Review Comment:
hard string.
Adding test for empty and seq stream should be better . I am not very similar with Mono.
--
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] chore: Add MonoToFuture to reduce conversion. [pekko-persistence-r2dbc]
Posted by "laglangyue (via GitHub)" <gi...@apache.org>.
laglangyue commented on PR #96:
URL: https://github.com/apache/pekko-persistence-r2dbc/pull/96#issuecomment-2016347393
LGTM
--
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] chore: Add MonoToFuture to reduce conversion. [pekko-persistence-r2dbc]
Posted by "He-Pin (via GitHub)" <gi...@apache.org>.
He-Pin commented on code in PR #96:
URL: https://github.com/apache/pekko-persistence-r2dbc/pull/96#discussion_r1535617359
##########
core/src/main/scala/org/apache/pekko/persistence/r2dbc/internal/MonoToFuture.scala:
##########
@@ -0,0 +1,59 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * license agreements; and to You under the Apache License, version 2.0:
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
Review Comment:
Will update later
--
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] chore: Add MonoToFuture to reduce conversion. [pekko-persistence-r2dbc]
Posted by "He-Pin (via GitHub)" <gi...@apache.org>.
He-Pin commented on code in PR #96:
URL: https://github.com/apache/pekko-persistence-r2dbc/pull/96#discussion_r1535828507
##########
core/src/test/scala/org/apache/pekko/persistence/r2dbc/internal/MonoToFutureSpec.scala:
##########
@@ -0,0 +1,27 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * license agreements; and to You under the Apache License, version 2.0:
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * This file is part of the Apache Pekko project, which was derived from Akka.
+ */
+
+package org.apache.pekko.persistence.r2dbc.internal
+
+import org.scalatest.TestSuite
+import org.scalatest.concurrent.ScalaFutures
+import org.scalatest.matchers.should.Matchers
+import org.scalatest.wordspec.AnyWordSpec
+import reactor.core.publisher.Mono
+
+class MonoToFutureSpec extends AnyWordSpec with ScalaFutures with TestSuite with Matchers {
+ "MonoToFutureSpec" should {
+ "convert a Mono to a Future" in {
+ val r = Mono.just("pekko")
Review Comment:
Mono is just some kind of Future, which can only emit one/zero elements.
--
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] chore: Add MonoToFuture to reduce conversion. [pekko-persistence-r2dbc]
Posted by "pjfanning (via GitHub)" <gi...@apache.org>.
pjfanning commented on PR #96:
URL: https://github.com/apache/pekko-persistence-r2dbc/pull/96#issuecomment-2015539815
Generally looks ok but I'd prefer not to merge this till we split off a 1.0.x branch.
--
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] chore: Add MonoToFuture to reduce conversion. [pekko-persistence-r2dbc]
Posted by "pjfanning (via GitHub)" <gi...@apache.org>.
pjfanning commented on code in PR #96:
URL: https://github.com/apache/pekko-persistence-r2dbc/pull/96#discussion_r1535528065
##########
core/src/main/scala/org/apache/pekko/persistence/r2dbc/internal/MonoToFuture.scala:
##########
@@ -0,0 +1,59 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * license agreements; and to You under the Apache License, version 2.0:
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
Review Comment:
is it possible that we should be using the standard ASF header here? this Akka derived header is only for Akka derived code
--
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] chore: Add MonoToFuture to reduce conversion. [pekko-persistence-r2dbc]
Posted by "laglangyue (via GitHub)" <gi...@apache.org>.
laglangyue commented on PR #96:
URL: https://github.com/apache/pekko-persistence-r2dbc/pull/96#issuecomment-2015195258
BTW, I have a question:
if a server provide a java-client using java completeFuture and a mono/flux client. Which should I use in pekko-streams api?
use completeFuture client, I can writer code `Source.future(completeFuture.asScala)` , or use the other like this 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
Re: [PR] chore: Add MonoToFuture to reduce conversion. [pekko-persistence-r2dbc]
Posted by "pjfanning (via GitHub)" <gi...@apache.org>.
pjfanning commented on code in PR #96:
URL: https://github.com/apache/pekko-persistence-r2dbc/pull/96#discussion_r1535528426
##########
core/src/test/scala/org/apache/pekko/persistence/r2dbc/internal/MonoToFutureSpec.scala:
##########
@@ -0,0 +1,27 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * license agreements; and to You under the Apache License, version 2.0:
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
Review Comment:
I think this should have the standard ASF header
--
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] chore: Add MonoToFuture to reduce conversion. [pekko-persistence-r2dbc]
Posted by "He-Pin (via GitHub)" <gi...@apache.org>.
He-Pin commented on PR #96:
URL: https://github.com/apache/pekko-persistence-r2dbc/pull/96#issuecomment-2015423874
@laglangyue You can convert a Publisher to Pekko's Source, and a Pekko's Sink as a Publisher too, I mostly using CompletableFuture at $Work, but will use Pekko Stream when it fit best.
--
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] chore: Add MonoToFuture to reduce conversion. [pekko-persistence-r2dbc]
Posted by "He-Pin (via GitHub)" <gi...@apache.org>.
He-Pin commented on PR #96:
URL: https://github.com/apache/pekko-persistence-r2dbc/pull/96#issuecomment-2016482817
Thanks.
--
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] chore: Add MonoToFuture to reduce conversion. [pekko-persistence-r2dbc]
Posted by "He-Pin (via GitHub)" <gi...@apache.org>.
He-Pin merged PR #96:
URL: https://github.com/apache/pekko-persistence-r2dbc/pull/96
--
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] chore: Add MonoToFuture to reduce conversion. [pekko-persistence-r2dbc]
Posted by "laglangyue (via GitHub)" <gi...@apache.org>.
laglangyue commented on code in PR #96:
URL: https://github.com/apache/pekko-persistence-r2dbc/pull/96#discussion_r1535638761
##########
core/src/test/scala/org/apache/pekko/persistence/r2dbc/internal/MonoToFutureSpec.scala:
##########
@@ -0,0 +1,27 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * license agreements; and to You under the Apache License, version 2.0:
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * This file is part of the Apache Pekko project, which was derived from Akka.
+ */
+
+package org.apache.pekko.persistence.r2dbc.internal
+
+import org.scalatest.TestSuite
+import org.scalatest.concurrent.ScalaFutures
+import org.scalatest.matchers.should.Matchers
+import org.scalatest.wordspec.AnyWordSpec
+import reactor.core.publisher.Mono
+
+class MonoToFutureSpec extends AnyWordSpec with ScalaFutures with TestSuite with Matchers {
+ "MonoToFutureSpec" should {
+ "convert a Mono to a Future" in {
+ val r = Mono.just("pekko")
Review Comment:
hard string.
Adding test for empty and seq stream should be better . I am not very familiarwith Mono.
##########
core/src/test/scala/org/apache/pekko/persistence/r2dbc/internal/MonoToFutureSpec.scala:
##########
@@ -0,0 +1,27 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * license agreements; and to You under the Apache License, version 2.0:
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * This file is part of the Apache Pekko project, which was derived from Akka.
+ */
+
+package org.apache.pekko.persistence.r2dbc.internal
+
+import org.scalatest.TestSuite
+import org.scalatest.concurrent.ScalaFutures
+import org.scalatest.matchers.should.Matchers
+import org.scalatest.wordspec.AnyWordSpec
+import reactor.core.publisher.Mono
+
+class MonoToFutureSpec extends AnyWordSpec with ScalaFutures with TestSuite with Matchers {
+ "MonoToFutureSpec" should {
+ "convert a Mono to a Future" in {
+ val r = Mono.just("pekko")
Review Comment:
hard string.
Adding test for empty and seq stream should be better . I am not very familiar with Mono.
--
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