You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@pekko.apache.org by "kczulko (via GitHub)" <gi...@apache.org> on 2024/02/06 09:50:45 UTC
[PR] [scala3] Enable scala3_sources [incubator-pekko-grpc]
kczulko opened a new pull request, #222:
URL: https://github.com/apache/incubator-pekko-grpc/pull/222
This PR attempts to enable `scala3_sources` scalapb option which is [described here](https://scalapb.github.io/docs/customizations#file-level-options).
--
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] Enable scala3_sources [incubator-pekko-grpc]
Posted by "pjfanning (via GitHub)" <gi...@apache.org>.
pjfanning commented on PR #222:
URL: https://github.com/apache/incubator-pekko-grpc/pull/222#issuecomment-1931588047
The Link Validator tests are very brittle. It calls all the linked sites and many of them fail on occasion.
The compatibility test is something we can probably rework or remove. Let me discuss it with other contributors.
--
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] Enable scala3_sources [incubator-pekko-grpc]
Posted by "pjfanning (via GitHub)" <gi...@apache.org>.
pjfanning commented on PR #222:
URL: https://github.com/apache/incubator-pekko-grpc/pull/222#issuecomment-1929153989
@kczulko will this break on our Scala 2.12 build?
--
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] Enable scala3_sources [incubator-pekko-grpc]
Posted by "pjfanning (via GitHub)" <gi...@apache.org>.
pjfanning commented on PR #222:
URL: https://github.com/apache/incubator-pekko-grpc/pull/222#issuecomment-1929218783
it doesn't yet work - it seems to break Scala 2.12 as I suspected it might https://github.com/apache/incubator-pekko-grpc/actions/runs/7797963372/job/21265803687?pr=222
--
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] Enable scala3_sources [incubator-pekko-grpc]
Posted by "kczulko (via GitHub)" <gi...@apache.org>.
kczulko commented on PR #222:
URL: https://github.com/apache/incubator-pekko-grpc/pull/222#issuecomment-1931568131
@pjfanning
Regarding failed test case... There's the [protobuf-javascript issue](https://github.com/protocolbuffers/protobuf-javascript/issues/127). Do want to workaround it somehow? It would require more work to get fixed. E.g. as a nixos user I added `protoc-gen-go` to my nix shell (so that it appears in PATH) and slightly modified 06-compatibility-plugins test case. With nix it's easy however if you'd like to avoid nix, then it required more code to determine developer's OS and whole download/unzip/(add-to-path | configure-build.sbt) logic.
Next thing - I don't know why `Link Validator` build step failed :)
--
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] Enable scala3_sources [incubator-pekko-grpc]
Posted by "raboof (via GitHub)" <gi...@apache.org>.
raboof commented on code in PR #222:
URL: https://github.com/apache/incubator-pekko-grpc/pull/222#discussion_r1495746889
##########
codegen/src/main/scala/org/apache/pekko/grpc/gen/scaladsl/ScalaCompatConstants.scala:
##########
@@ -0,0 +1,23 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.pekko.grpc.gen.scaladsl
+
+private[scaladsl] class ScalaCompatConstants(emitScala3Sources: Boolean = false) {
+ // val WildcardType: String = if (emitScala3Sources) "?" else "_"
Review Comment:
I guess this can 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
Re: [PR] [scala3] Enable scala3_sources [incubator-pekko-grpc]
Posted by "samueleresca (via GitHub)" <gi...@apache.org>.
samueleresca commented on code in PR #222:
URL: https://github.com/apache/incubator-pekko-grpc/pull/222#discussion_r1495768653
##########
sbt-plugin/src/sbt-test/gen-scala-server/06-compatibility-plugins/project/ProtocJSPlugin.scala:
##########
@@ -19,5 +19,7 @@ object ProtocJSPlugin extends AutoPlugin {
override def requires: Plugins = ProtocPlugin
override def projectSettings: Seq[Def.Setting[_]] =
- Seq(Compile, Test).flatMap(inConfig(_)(PB.targets += PB.gens.js -> resourceManaged.value / "js"))
+ Seq(Compile, Test).flatMap(inConfig(_)(
+ Seq(
+ PB.targets += PB.gens.go -> resourceManaged.value / "go")))
Review Comment:
Should the `ProtocJSPlugin` object renamed to something `ProtocGoPlugin`?
--
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] Enable scala3_sources [incubator-pekko-grpc]
Posted by "kczulko (via GitHub)" <gi...@apache.org>.
kczulko commented on PR #222:
URL: https://github.com/apache/incubator-pekko-grpc/pull/222#issuecomment-1954221397
Dear Lord... I think it's ok now :D
--
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] Enable scala3_sources [incubator-pekko-grpc]
Posted by "raboof (via GitHub)" <gi...@apache.org>.
raboof commented on code in PR #222:
URL: https://github.com/apache/incubator-pekko-grpc/pull/222#discussion_r1495750001
##########
runtime/src/main/scala/org/apache/pekko/grpc/internal/PekkoDiscoveryNameResolverProvider.scala:
##########
@@ -34,8 +35,7 @@ class PekkoDiscoveryNameResolverProvider(
override def getDefaultScheme: String = "http"
- override def newNameResolver(targetUri: URI, args: NameResolver.Args): PekkoDiscoveryNameResolver = {
- require(targetUri.getAuthority != null, s"target uri should not have null authority, got [$targetUri]")
- new PekkoDiscoveryNameResolver(discovery, defaultPort, targetUri.getAuthority, portName, protocol, resolveTimeout)
- }
+ override def newNameResolver(targetUri: URI, args: NameResolver.Args): PekkoDiscoveryNameResolver =
+ new PekkoDiscoveryNameResolver(discovery, defaultPort, serviceName, portName, protocol, resolveTimeout)
Review Comment:
Is this change related/required for the switch to the new io.grpc version and introduction of scala3_sources?
--
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] Enable scala3_sources [incubator-pekko-grpc]
Posted by "raboof (via GitHub)" <gi...@apache.org>.
raboof commented on PR #222:
URL: https://github.com/apache/incubator-pekko-grpc/pull/222#issuecomment-1938338383
> With nix it's easy however if you'd like to avoid nix, then it requires more code to determine developer's OS and whole download/unzip/(add-to-path | configure-build.sbt) logic. Or some shenanigans could be done on the .github workflow files but I would rather want to keep coherent configuration for both local and CI envs
since this is an sbt integration test, I think it's OK if there's some prerequisites for running it (such as putting `protoc-gen-go` on the PATH). If we can add that to the GHA script and document that the user needs to do that themselves (however it needs to be done on their OS), then I think we're OK.
--
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] Enable scala3_sources [incubator-pekko-grpc]
Posted by "raboof (via GitHub)" <gi...@apache.org>.
raboof merged PR #222:
URL: https://github.com/apache/incubator-pekko-grpc/pull/222
--
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] Enable scala3_sources [incubator-pekko-grpc]
Posted by "raboof (via GitHub)" <gi...@apache.org>.
raboof commented on PR #222:
URL: https://github.com/apache/incubator-pekko-grpc/pull/222#issuecomment-1954192402
> > the GHA error seems to be 'every step must define a `uses` or `run` key' - though this is strange since it seems you do.
>
> I think that error is caused by the `Install go & go-protobuf` step defined with 2 "-":
>
> ```
> - name: Install go & go-protobuf
> - uses: actions/setup-go@v5
> ...
> ```
ah, indeed, sharp eyes!
--
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] Enable scala3_sources [incubator-pekko-grpc]
Posted by "kczulko (via GitHub)" <gi...@apache.org>.
kczulko commented on PR #222:
URL: https://github.com/apache/incubator-pekko-grpc/pull/222#issuecomment-1929157411
I don't think so. We're using both scala2 and scala3 in our project (event the same sbt module is being compiled twice, due to porting-to-scala3 process...). I've been checking this via `+publishLocal` and all was fine. Also tests were passed.
--
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] Enable scala3_sources [incubator-pekko-grpc]
Posted by "raboof (via GitHub)" <gi...@apache.org>.
raboof commented on code in PR #222:
URL: https://github.com/apache/incubator-pekko-grpc/pull/222#discussion_r1495751591
##########
interop-tests/src/test/scala/org/apache/pekko/grpc/scaladsl/NonBalancingIntegrationSpec.scala:
##########
@@ -184,8 +183,7 @@ class NonBalancingIntegrationSpec(backend: String)
val failure =
client.sayHello(HelloRequest(s"Hello friend")).failed.futureValue.asInstanceOf[StatusRuntimeException]
- failure.getStatus.getCode should be(Code.UNAVAILABLE)
- client.closed.failed.futureValue shouldBe a[ClientConnectionException]
+ failure.getStatus.getCode should (equal(Code.UNKNOWN).or(equal(Code.UNAVAILABLE)))
Review Comment:
strange but no problem
--
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] Enable scala3_sources [incubator-pekko-grpc]
Posted by "raboof (via GitHub)" <gi...@apache.org>.
raboof commented on code in PR #222:
URL: https://github.com/apache/incubator-pekko-grpc/pull/222#discussion_r1495869528
##########
runtime/src/main/scala/org/apache/pekko/grpc/internal/PekkoDiscoveryNameResolverProvider.scala:
##########
@@ -34,8 +35,7 @@ class PekkoDiscoveryNameResolverProvider(
override def getDefaultScheme: String = "http"
- override def newNameResolver(targetUri: URI, args: NameResolver.Args): PekkoDiscoveryNameResolver = {
- require(targetUri.getAuthority != null, s"target uri should not have null authority, got [$targetUri]")
- new PekkoDiscoveryNameResolver(discovery, defaultPort, targetUri.getAuthority, portName, protocol, resolveTimeout)
- }
+ override def newNameResolver(targetUri: URI, args: NameResolver.Args): PekkoDiscoveryNameResolver =
+ new PekkoDiscoveryNameResolver(discovery, defaultPort, serviceName, portName, protocol, resolveTimeout)
Review Comment:
Hmm, that is slightly scary (I assume the assert was there for a reason), but if you're confident this is a correct fix then :+1:
--
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] Enable scala3_sources [incubator-pekko-grpc]
Posted by "pjfanning (via GitHub)" <gi...@apache.org>.
pjfanning commented on PR #222:
URL: https://github.com/apache/incubator-pekko-grpc/pull/222#issuecomment-1932299826
@mdedetrich is it ok to remove the compatibility test that is failing and log a new issue to replace it? grpc refactored the javascript support and the test needs to be rewritten or replaced if we want to upgrade grpc libs.
--
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] Enable scala3_sources [incubator-pekko-grpc]
Posted by "kczulko (via GitHub)" <gi...@apache.org>.
kczulko commented on PR #222:
URL: https://github.com/apache/incubator-pekko-grpc/pull/222#issuecomment-1935714480
@pjfanning @mdedetrich
Please let me know if there's anything you want from me to push it forward.
Thanks in advance,
Karol
--
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] Enable scala3_sources [incubator-pekko-grpc]
Posted by "raboof (via GitHub)" <gi...@apache.org>.
raboof commented on PR #222:
URL: https://github.com/apache/incubator-pekko-grpc/pull/222#issuecomment-1954175021
the GHA error seems to be 'every step must define a `uses` or `run` key' - though this is strange since it seems you do.
--
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] Enable scala3_sources [incubator-pekko-grpc]
Posted by "raboof (via GitHub)" <gi...@apache.org>.
raboof commented on code in PR #222:
URL: https://github.com/apache/incubator-pekko-grpc/pull/222#discussion_r1495751099
##########
runtime/src/main/mima-filters/1.1.x.backwards.excludes/io.grpc-upgrade.backwards.excludes:
##########
@@ -0,0 +1,19 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# Upgrade to io.grpc 1.60 caused this bin compat issue
+ProblemFilters.exclude[DirectMissingMethodProblem]("org.apache.pekko.grpc.internal.PekkoDiscoveryNameResolverProvider.this")
Review Comment:
:+1: since internal
--
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] Enable scala3_sources [incubator-pekko-grpc]
Posted by "raboof (via GitHub)" <gi...@apache.org>.
raboof commented on PR #222:
URL: https://github.com/apache/incubator-pekko-grpc/pull/222#issuecomment-1954131861
Great work here!
I'm confused why the `Validate and test` CI workflow (from `build-test.yml`) isn't running 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
Re: [PR] [scala3] Enable scala3_sources [incubator-pekko-grpc]
Posted by "samueleresca (via GitHub)" <gi...@apache.org>.
samueleresca commented on PR #222:
URL: https://github.com/apache/incubator-pekko-grpc/pull/222#issuecomment-1954191019
> the GHA error seems to be 'every step must define a `uses` or `run` key' - though this is strange since it seems you do.
I think that error is caused by the `Install go & go-protobuf` step defined with 2 "-":
```
- name: Install go & go-protobuf
- uses: actions/setup-go@v5
...
```
--
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] Enable scala3_sources [incubator-pekko-grpc]
Posted by "mdedetrich (via GitHub)" <gi...@apache.org>.
mdedetrich commented on PR #222:
URL: https://github.com/apache/incubator-pekko-grpc/pull/222#issuecomment-1929404301
> > Probably need to pass some flags to scalacOptions so that the scala 2.12 compiler can accept the scala 3 source style
>
> The errors are coming from java files. For now I don't get it what could cause this issue. Do you have some suggestions?
So that file is generated by googles own protobuf compiler so you shouldn't even manually be modifying it. Maybe we need to update the google protobuf compiler as well?
--
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] Enable scala3_sources [incubator-pekko-grpc]
Posted by "kczulko (via GitHub)" <gi...@apache.org>.
kczulko commented on PR #222:
URL: https://github.com/apache/incubator-pekko-grpc/pull/222#issuecomment-1929415366
As I'm bumping the scalapb compilerplugin I suspect the incompatibility may be here? https://github.com/scalapb/ScalaPB/blob/71befffd3db412d158fabb807ded56920fa0030f/project/Dependencies.scala#L7
--
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] Enable scala3_sources [incubator-pekko-grpc]
Posted by "kczulko (via GitHub)" <gi...@apache.org>.
kczulko commented on code in PR #222:
URL: https://github.com/apache/incubator-pekko-grpc/pull/222#discussion_r1495800060
##########
runtime/src/main/scala/org/apache/pekko/grpc/internal/PekkoDiscoveryNameResolverProvider.scala:
##########
@@ -34,8 +35,7 @@ class PekkoDiscoveryNameResolverProvider(
override def getDefaultScheme: String = "http"
- override def newNameResolver(targetUri: URI, args: NameResolver.Args): PekkoDiscoveryNameResolver = {
- require(targetUri.getAuthority != null, s"target uri should not have null authority, got [$targetUri]")
- new PekkoDiscoveryNameResolver(discovery, defaultPort, targetUri.getAuthority, portName, protocol, resolveTimeout)
- }
+ override def newNameResolver(targetUri: URI, args: NameResolver.Args): PekkoDiscoveryNameResolver =
+ new PekkoDiscoveryNameResolver(discovery, defaultPort, serviceName, portName, protocol, resolveTimeout)
Review Comment:
It looks like `authority` is no longer passed when `newNameResolver` is called.
--
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] Enable scala3_sources [incubator-pekko-grpc]
Posted by "raboof (via GitHub)" <gi...@apache.org>.
raboof commented on code in PR #222:
URL: https://github.com/apache/incubator-pekko-grpc/pull/222#discussion_r1495812430
##########
.github/workflows/build-test.yml:
##########
@@ -110,6 +110,12 @@ jobs:
distribution: temurin
java-version: 8
+ - name: Install go
+ uses: actions/setup-go@v5
+ with:
+ go-version: '^1.20'
+ run: go install google.golang.org/protobuf/cmd/protoc-gen-go@v1.32.0
+
Review Comment:
Ah looks like we have to split this into two: "a step cannot have both the `uses` and `run` keys"
--
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] Enable scala3_sources [incubator-pekko-grpc]
Posted by "raboof (via GitHub)" <gi...@apache.org>.
raboof commented on PR #222:
URL: https://github.com/apache/incubator-pekko-grpc/pull/222#issuecomment-1954240007
It seems the link validator isn't wrong here, and the license report is linking to http://www.jetbrains.org for a new `org.jetbrains:annotations:13.0` dependency. We might want to add http://www.jetbrains.org to the `non-https-whitelist` in `./scripts/link-validator.conf`?
--
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] Enable scala3_sources [incubator-pekko-grpc]
Posted by "kczulko (via GitHub)" <gi...@apache.org>.
kczulko commented on PR #222:
URL: https://github.com/apache/incubator-pekko-grpc/pull/222#issuecomment-1929394361
> Probably need to pass some flags to scalacOptions so that the scala 2.12 compiler can accept the scala 3 source style
The errors are coming from java files. For now I don't get it what could cause this issue. Do you have some suggestions?
--
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] Enable scala3_sources [incubator-pekko-grpc]
Posted by "mdedetrich (via GitHub)" <gi...@apache.org>.
mdedetrich commented on PR #222:
URL: https://github.com/apache/incubator-pekko-grpc/pull/222#issuecomment-1929185272
My gut feeling is that this should be targeting 1.1.x, @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] Enable scala3_sources [incubator-pekko-grpc]
Posted by "pjfanning (via GitHub)" <gi...@apache.org>.
pjfanning commented on PR #222:
URL: https://github.com/apache/incubator-pekko-grpc/pull/222#issuecomment-1929472757
yes - a big bump in io.grpc libs causes us issues - eg https://github.com/apache/incubator-pekko-grpc/pull/190
--
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] Enable scala3_sources [incubator-pekko-grpc]
Posted by "mdedetrich (via GitHub)" <gi...@apache.org>.
mdedetrich commented on PR #222:
URL: https://github.com/apache/incubator-pekko-grpc/pull/222#issuecomment-1929221967
> it doesn't yet work - it seems to break Scala 2.12 as I suspected it might https://github.com/apache/incubator-pekko-grpc/actions/runs/7797963372/job/21265803687?pr=222
Probably need to pass some flags to `scalacOptions` so that the scala 2.12 compiler can accept the scala 3 source style
--
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] Enable scala3_sources [incubator-pekko-grpc]
Posted by "kczulko (via GitHub)" <gi...@apache.org>.
kczulko commented on PR #222:
URL: https://github.com/apache/incubator-pekko-grpc/pull/222#issuecomment-1942156862
@raboof
Ok then. Let me try to do it. I'll use [this gh action](https://github.com/actions/setup-go) together with [this go install command](https://protobuf.dev/getting-started/gotutorial/#compiling-protocol-buffers). However, I'm not allowed to check how this will work under github actions env since contributors have to wait for the CI run approval. Therefore I'd like to ask you (the contributors) to eventually fix all the shortcomings (as you can probably do it without waiting for approvals).
Best regards,
Karol
--
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