upgrade to jackson 2.14.2 #273

pjfanning · 2023-03-27T13:29:41Z

Jackson 2.14.2 for Scala 3 - supports only Scala 3.2 - so we'll need to upgrade Scala
There is a deprecation warning - a Jackson method that we use that has now been deprecated.
The deprecated method needs a large scale rewrite to do things the preferred Jackson 2.14 way
Jackson is heading towards making ObjectMapper immutable and the preferred approach is to use JsonMapper.builder()... to create a configured MapperBuilder and to create JsonMapper/ObjectMapper instances from it
In short term, we are probably bestter off not moving off the deprecated method - it won't be removed for ages

[error] /home/runner/work/incubator-pekko/incubator-pekko/serialization-jackson/src/main/scala/org/apache/pekko/serialization/jackson/JacksonObjectMapperProvider.scala:170:45: method configure in class ObjectMapper is deprecated
[error]       case (feature, value) => objectMapper.configure(feature, value)
[error]                                             ^
[error] one error found

nvollmar

lgtm

pjfanning · 2023-03-27T14:14:17Z

@mdedetrich any idea how to get the Scala 3 build to ignore deprecation issues? https://github.com/apache/incubator-pekko/actions/runs/4532777359/jobs/7984657231?pr=273 is failing, seems to be because it won't allow deprecated code in the build

mdedetrich · 2023-03-27T14:17:05Z

@pjfanning

@mdedetrich any idea how to get the Scala 3 build to ignore deprecation issues? https://github.com/apache/incubator-pekko/actions/runs/4532777359/jobs/7984657231?pr=273 is failing, seems to be because it won't allow deprecated code in the build

How about adding @nowarn(cat="msg=deprecated") on the function where its being used?

mdedetrich · 2023-03-27T14:19:17Z

Alternately you can just copy https://github.com/apache/incubator-pekko/pull/270/files#diff-676691b2e85cad6026adb107942b7c9325394e20e015b131fdcebd3b2b7794b0R137 from my Scala 3.3 PR, that will just globally disable any deprecation warnings in the Scala3 build (which we will probably have to do in the future anyways).

mdedetrich · 2023-03-27T14:37:37Z

@pjfanning For the latest error from https://github.com/apache/incubator-pekko/actions/runs/4533101420/jobs/7985451216?pr=273 you probably need to add serialization-jackson into PekkoDisciplinePlugin.nonFatalJavaWarningsFor (along with a comment explaining why). The same error is also occurring in Scala 2.12

mdedetrich · 2023-03-27T14:48:02Z

@pjfanning Okay so for the new errors in https://github.com/apache/incubator-pekko/actions/runs/4533274521/jobs/7985858691?pr=273 you probably need to port some of the fixes from #270, specifically the @unchecked ones

pjfanning · 2023-03-27T15:19:21Z

@pjfanning Okay so for the new errors in https://github.com/apache/incubator-pekko/actions/runs/4533274521/jobs/7985858691?pr=273 you probably need to port some of the fixes from #270, specifically the @unchecked ones

Thanks @mdedetrich - it looks like I needed all your code changes from your Scala 3.3 PR

mdedetrich

So I am going to approve this PR but since there was some ambivalence about upgrading the Scala version (see #6 and https://lists.apache.org/thread/mk88ybzx51k7x7x102drtbzh6l68k0xm) we should probably check with community that this is broadly okay before merging (as people may already know, personally I am all for updating the Scala 3 version but thats just me)

jrudolph

In general, lgtm.

I don't quite see how the general silencing and the @nowarn and @unchecked annotation changes play together here. Can we find a way to silence exactly the deprecation warnings we are concerned about and only in that submodule?

jrudolph · 2023-04-04T09:50:50Z

project/PekkoDisciplinePlugin.scala

@@ -134,7 +136,7 @@ object PekkoDisciplinePlugin extends AutoPlugin {
          case Some((2, 12)) =>
            disciplineScalacOptions
          case _ =>
-            Nil
+            Seq("-Wconf:cat=deprecation:s")


Does that mean we disable all deprecation warnings for Scala 3 for all submodules. Is that really what we want?

You can also match by other factors, so maybe we can restrict the silencing to Jackson in particular?

To provide some context, this flag is needed for Scala 3.2.x in general, not Jackson specifically. It appears there was an issue in Scala 3.1.x where it wasn't reporting deprecations as it should and this was fixed in Scala 3.2. I also had to the do the same fix in the PR I created that only updates to Scala 3.3 (with no other changes), see https://github.com/apache/incubator-pekko/pull/270/files#diff-676691b2e85cad6026adb107942b7c9325394e20e015b131fdcebd3b2b7794b0R137.

actor-typed-tests/src/test/scala/docs/org/apache/pekko/typed/InteractionPatternsSpec.scala

mdedetrich · 2023-04-04T10:20:43Z

I don't quite see how the general silencing and the @nowarn and @unchecked annotation changes play together here. Can we find a way to silence exactly the deprecation warnings we are concerned about and only in that submodule?

@jrudolph So the changes here were copied from my work in Scala 3.3 (see #270) because it turns out all of the Scala 3 specific changes done in the Scala 3.3 PR due to Scala 3 update was actually added in Scala 3.2 (which this PR is updating to).

With the deprecation warnings, there was a lot of places that Scala 3.2/3.3 was warning about deprecation so at least in my Scala 3.3 PR I made the decision to add the flag in order to reduce noise. While it was possible to use the @nowarn flag to silence the deprecation, these deprecation warnings was being generated in code that is being shared between Scala 2 and Scala 3 which means that if we were to use the @nowarn flag we would have to do changes to the Scala 2 compiler flags in order to not complain if no warnings were silenced (latest versions of Scala 2.12/2.13 will generate a warning if you use a @nowarn annotation that doesn't actually suppress anything, and iirc these deprecations warnings are Scala 3.2/3.3 specific and don't occur in Scala 2.12/2.13)

Regarding @unchecked, this seems to be due to an improvement that landed in Scala 3.2 which improves the exhaustiveness checking for pattern matching cases. Originally in my Scala 3.3 PR I tried to find a flag which covers only these new cases that require @unchecked so it can be silenced but it appears that the change in Scala 3.2 is extending an already existing case of exhaustiveness checking.

…nteractionPatternsSpec.scala Co-authored-by: Johannes Rudolph <[email protected]>

pjfanning · 2023-04-05T11:26:46Z

@mdedetrich @jrudolph are there further changes needed here or is it safe to proceed with a merge?

jrudolph · 2023-04-05T12:56:57Z

Thanks for the explanation, @mdedetrich. The whole linting business here is quite a mess anyway, so it's ok for me to go ahead. Maybe create a ticket to revisit linting rules for Scala 3 (or overall...).

jrudolph

LGTM

mdedetrich

Lgtm

upgrade to jackson 2.14.2

4e592e1

nvollmar approved these changes Mar 27, 2023

View reviewed changes

pjfanning added 2 commits March 27, 2023 16:00

scala 3.2.2 needed

37b3620

another deprecation issue

778b7e6

ignore deprecation warnings in Scala 3 build

b0a69ae

Update PekkoDisciplinePlugin.scala

96d53a2

pjfanning added 2 commits March 27, 2023 17:01

scala 3.2 compile issues

5165092

more scala upgrade issues

b27298c

mdedetrich mentioned this pull request Mar 27, 2023

Upgrade Scala 3.x versions #6

Closed

mdedetrich approved these changes Mar 27, 2023

View reviewed changes

import order

235d137

mdedetrich mentioned this pull request Apr 3, 2023

ensure scala-3 branch ends up with all the changes on main branch apache/pekko-http#17

Closed

danischroeter approved these changes Apr 3, 2023

View reviewed changes

jrudolph reviewed Apr 4, 2023

View reviewed changes

Update actor-typed-tests/src/test/scala/docs/org/apache/pekko/typed/I…

bd12027

…nteractionPatternsSpec.scala Co-authored-by: Johannes Rudolph <[email protected]>

jrudolph approved these changes Apr 5, 2023

View reviewed changes

mdedetrich approved these changes Apr 5, 2023

View reviewed changes

pjfanning merged commit 321c572 into apache:main Apr 5, 2023

pjfanning deleted the jackson-2.14 branch April 5, 2023 13:45

This was referenced Apr 5, 2023

upgrade to jackson 2.14.2 apache/pekko-http#131

Merged

Scala 3 support apache/pekko-http#130

Closed

upgrade to jackson 2.14.2 #273

upgrade to jackson 2.14.2 #273

Uh oh!

Conversation

pjfanning commented Mar 27, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nvollmar left a comment

Choose a reason for hiding this comment

Uh oh!

pjfanning commented Mar 27, 2023

Uh oh!

mdedetrich commented Mar 27, 2023

Uh oh!

mdedetrich commented Mar 27, 2023

Uh oh!

mdedetrich commented Mar 27, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mdedetrich commented Mar 27, 2023

Uh oh!

pjfanning commented Mar 27, 2023

Uh oh!

mdedetrich left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jrudolph left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jrudolph Apr 4, 2023

Choose a reason for hiding this comment

Uh oh!

mdedetrich Apr 4, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mdedetrich commented Apr 4, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pjfanning commented Apr 5, 2023

Uh oh!

jrudolph commented Apr 5, 2023

Uh oh!

jrudolph left a comment

Choose a reason for hiding this comment

Uh oh!

mdedetrich left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

pjfanning commented Mar 27, 2023 •

edited

Loading

mdedetrich commented Mar 27, 2023 •

edited

Loading

mdedetrich left a comment •

edited

Loading

jrudolph left a comment •

edited

Loading

mdedetrich commented Apr 4, 2023 •

edited

Loading