You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@pekko.apache.org by "mdedetrich (via GitHub)" <gi...@apache.org> on 2023/05/08 19:48:42 UTC
[GitHub] [incubator-pekko-http] mdedetrich opened a new pull request, #142: Apply compat changes from latest Pekko
mdedetrich opened a new pull request, #142:
URL: https://github.com/apache/incubator-pekko-http/pull/142
TBD
--
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-http] mdedetrich commented on a diff in pull request #142: Apply compat changes from latest Pekko
Posted by "mdedetrich (via GitHub)" <gi...@apache.org>.
mdedetrich commented on code in PR #142:
URL: https://github.com/apache/incubator-pekko-http/pull/142#discussion_r1188685913
##########
http-core/src/main/java/org/apache/pekko/http/javadsl/model/headers/CacheDirectives.java:
##########
@@ -28,7 +28,7 @@ public static CacheDirective MAX_STALE() {
return new org.apache.pekko.http.scaladsl.model.headers.CacheDirectives.max$minusstale(OptionConverters.toScala(Optional.empty()));
}
public static CacheDirective MAX_STALE(long deltaSeconds) {
- return new org.apache.pekko.http.scaladsl.model.headers.CacheDirectives.max$minusstale(OptionConverters.toScala(OptionalLong.of(deltaSeconds)));
+ return new org.apache.pekko.http.scaladsl.model.headers.CacheDirectives.max$minusstale(new scala.Some(deltaSeconds));
Review Comment:
Since `deltaSeconds` cannot be null (its a Java primitive) we can just directly create a `scala.Some` here.
--
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-http] mdedetrich commented on a diff in pull request #142: Apply compat changes from latest Pekko
Posted by "mdedetrich (via GitHub)" <gi...@apache.org>.
mdedetrich commented on code in PR #142:
URL: https://github.com/apache/incubator-pekko-http/pull/142#discussion_r1188701971
##########
http/src/main/scala/org/apache/pekko/http/javadsl/server/RoutingJavaMapping.scala:
##########
@@ -99,7 +99,7 @@ private[http] object RoutingJavaMapping {
// val javaToScalaResponseEntity extends Inherited[javadsl.model.ResponseEntity, scaladsl.model.ResponseEntity]
implicit final class ConvertCompletionStage[T](val stage: CompletionStage[T]) extends AnyVal {
- import scala.compat.java8.FutureConverters._
- def asScala = stage.toScala
+ import pekko.util.FutureConverters
Review Comment:
This was changed from using `.asScala` to `FutureConverters.asScala(stage)` because the `asScala` method from `pekko.util.FutureConverters._` was conflicting with an `org.apache.pekko.http.impl.util.AsScala.asScala` implicit.
--
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-http] mdedetrich commented on a diff in pull request #142: Apply compat changes from latest Pekko
Posted by "mdedetrich (via GitHub)" <gi...@apache.org>.
mdedetrich commented on code in PR #142:
URL: https://github.com/apache/incubator-pekko-http/pull/142#discussion_r1188703315
##########
http/src/main/scala/org/apache/pekko/http/javadsl/server/directives/CacheConditionDirectives.scala:
##########
@@ -85,7 +83,7 @@ abstract class CacheConditionDirectives extends BasicDirectives {
*/
def conditional(eTag: Optional[EntityTag], lastModified: Optional[DateTime], inner: Supplier[Route]): Route =
RouteAdapter {
- D.conditional(eTag.asScala.map(_.asScala), lastModified.asScala.map(_.asScala)) { inner.get.delegate }
+ D.conditional(eTag.asScala, lastModified.asScala) { inner.get.delegate }
Review Comment:
The `.map(_.asScala)` appeared to be entirely pointless here.
--
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-http] mdedetrich merged pull request #142: Apply compat changes from latest Pekko
Posted by "mdedetrich (via GitHub)" <gi...@apache.org>.
mdedetrich merged PR #142:
URL: https://github.com/apache/incubator-pekko-http/pull/142
--
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-http] mdedetrich commented on a diff in pull request #142: Apply compat changes from latest Pekko
Posted by "mdedetrich (via GitHub)" <gi...@apache.org>.
mdedetrich commented on code in PR #142:
URL: https://github.com/apache/incubator-pekko-http/pull/142#discussion_r1188695749
##########
http-core/src/main/scala/org/apache/pekko/http/javadsl/settings/ServerSettings.scala:
##########
@@ -71,7 +70,7 @@ import scala.concurrent.duration.{ Duration, FiniteDuration }
// ---
def withServerHeader(newValue: Optional[Server]): ServerSettings =
- self.copy(serverHeader = newValue.asScala.map(_.asScala))
Review Comment:
The `map` here was entirely redundant as it seemed to be calling an implicit `asScala` that just returned itself
--
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-http] mdedetrich commented on a diff in pull request #142: Apply compat changes from latest Pekko
Posted by "mdedetrich (via GitHub)" <gi...@apache.org>.
mdedetrich commented on code in PR #142:
URL: https://github.com/apache/incubator-pekko-http/pull/142#discussion_r1187844965
##########
http-core/src/main/java/org/apache/pekko/http/javadsl/model/RemoteAddress.java:
##########
@@ -31,7 +30,7 @@ public abstract class RemoteAddress {
public abstract int getPort();
public static RemoteAddress create(InetAddress address) {
- return org.apache.pekko.http.scaladsl.model.RemoteAddress.apply(address, OptionConverters.toScala(Optional.empty()));
+ return org.apache.pekko.http.scaladsl.model.RemoteAddress.apply(address, Util.convertOptionalToScala(Optional.empty()));
Review Comment:
Yes I am aware of this, I was trying to get the entire thing to compile. Setting up a PR in pekko core to add this method 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
[GitHub] [incubator-pekko-http] pjfanning commented on a diff in pull request #142: Apply compat changes from latest Pekko
Posted by "pjfanning (via GitHub)" <gi...@apache.org>.
pjfanning commented on code in PR #142:
URL: https://github.com/apache/incubator-pekko-http/pull/142#discussion_r1187838180
##########
http-core/src/main/java/org/apache/pekko/http/javadsl/model/RemoteAddress.java:
##########
@@ -31,7 +30,7 @@ public abstract class RemoteAddress {
public abstract int getPort();
public static RemoteAddress create(InetAddress address) {
- return org.apache.pekko.http.scaladsl.model.RemoteAddress.apply(address, OptionConverters.toScala(Optional.empty()));
+ return org.apache.pekko.http.scaladsl.model.RemoteAddress.apply(address, Util.convertOptionalToScala(Optional.empty()));
Review Comment:
this seems like a very roundabout way to get `scala.None` - with all the extra processing overhead of creating RichOptionals and converting them.
--
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-http] mdedetrich commented on a diff in pull request #142: Apply compat changes from latest Pekko
Posted by "mdedetrich (via GitHub)" <gi...@apache.org>.
mdedetrich commented on code in PR #142:
URL: https://github.com/apache/incubator-pekko-http/pull/142#discussion_r1187832300
##########
docs/src/test/java/docs/http/javadsl/server/directives/FutureDirectivesExamplesTest.java:
##########
@@ -30,7 +30,6 @@
import scala.concurrent.duration.FiniteDuration;
import static org.apache.pekko.http.javadsl.server.PathMatchers.*;
-import static scala.compat.java8.JFunction.func;
Review Comment:
No longer needed, Scala 2.12+ supported automatic conversion of Scala functions to Java functions. This appears to be leftover from Scala 2.11 and earlier
--
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-http] mdedetrich commented on a diff in pull request #142: Apply compat changes from latest Pekko
Posted by "mdedetrich (via GitHub)" <gi...@apache.org>.
mdedetrich commented on code in PR #142:
URL: https://github.com/apache/incubator-pekko-http/pull/142#discussion_r1188684790
##########
http-core/src/main/java/org/apache/pekko/http/impl/util/Util.java:
##########
@@ -52,6 +54,26 @@ public static <T, U extends T> scala.Option<U> convertOptionalToScala(Optional<T
return OptionConverters.toScala((Optional<U>) o);
}
+ // This is needed to be used in Java source code that calls Scala code which expects scala.Long
Review Comment:
Doing this kind of annoyed me. For context this was meant to be solved within `org.apache.pekko.util.OptionConverters` via this PR https://github.com/apache/incubator-pekko/pull/316 however it turns out that `java.lang.Long`/`java.lang.Int` cannot be implicitly converted to `scala.Long`/`scala.Int` when written within Java source code.
While this can be abstracted away for all Scala versions within `org.apache.pekko.util.OptionConverters` the whole reason why the `java.lang.Long`/`java.lang.Int` methods were added in that PR is because the `scala.jdk.javaapi.OptionConverters` return `java.lang.Long`/`java.lang.Int` and its `scala.jdk.javaapi.OptionConverters` which will be used when Scala 2.12 is going to be dropped.
--
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-http] mdedetrich commented on a diff in pull request #142: Apply compat changes from latest Pekko
Posted by "mdedetrich (via GitHub)" <gi...@apache.org>.
mdedetrich commented on code in PR #142:
URL: https://github.com/apache/incubator-pekko-http/pull/142#discussion_r1188692383
##########
http-core/src/main/scala/org/apache/pekko/http/javadsl/ServerBuilder.scala:
##########
@@ -174,27 +174,27 @@ object ServerBuilder {
def bind(handler: Function[HttpRequest, CompletionStage[HttpResponse]]): CompletionStage[ServerBinding] =
http.bindAndHandleAsyncImpl(
- handler.apply(_).asScala,
+ handler.apply(_).asScala.map(_.asScala),
Review Comment:
The `.asScala` method here was conflict with the `org.apache.pekko.http.impl.util.AsScala.asScala` so I just did the conversion here directly.
--
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-http] mdedetrich commented on a diff in pull request #142: Apply compat changes from latest Pekko
Posted by "mdedetrich (via GitHub)" <gi...@apache.org>.
mdedetrich commented on code in PR #142:
URL: https://github.com/apache/incubator-pekko-http/pull/142#discussion_r1188702507
##########
http/src/main/scala/org/apache/pekko/http/javadsl/server/directives/BasicDirectives.scala:
##########
@@ -170,7 +174,7 @@ abstract class BasicDirectives {
def recoverRejectionsWith(
f: JFunction[JIterable[Rejection], CompletionStage[RouteResult]], inner: Supplier[Route]): Route = RouteAdapter {
D.recoverRejectionsWith(rs =>
- f.apply(Util.javaArrayList(rs.map(_.asJava))).toScala.fast.map(_.asScala)(
+ FutureConverters.asScala(f.apply(Util.javaArrayList(rs.map(_.asJava)))).fast.map(_.asScala)(
Review Comment:
This was changed from using `.asScala` to `FutureConverters.asScala(stage)` because the `asScala` method from `pekko.util.FutureConverters._` was conflicting with an `org.apache.pekko.http.impl.util.AsScala.asScala` implicit.
--
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