typelevel / catbird

Birds and cats together
http://catbird.io
Apache License 2.0
139 stars 22 forks source link

Update Finagle to 22.12.0 #521

Closed milanvdm closed 1 year ago

milanvdm commented 1 year ago

See https://github.com/twitter/finagle/releases

codecov-commenter commented 1 year ago

Codecov Report

Merging #521 (bf563b7) into main (78cd0ec) will not change coverage. The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #521   +/-   ##
=======================================
  Coverage   90.60%   90.60%           
=======================================
  Files          18       18           
  Lines         298      298           
  Branches        2        2           
=======================================
  Hits          270      270           
  Misses         28       28           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

bpholt commented 1 year ago

I'm kind of surprised there were no breaking changes in this release. Can we double-check that the MiMa "extra projects" are being checked correctly? If so, this lgtm.

milanvdm commented 1 year ago

@bpholt Indeed, the MiMa isnt set up correctly and doest run on the rootFinagle project. Will check :)

milanvdm commented 1 year ago
sbt:rootFinagle> mimaReportBinaryIssues
[info] rootFinagle: mimaPreviousArtifacts is empty, not analyzing binary compatibility.
[error] finagle-core: Failed binary compatibility check against com.twitter:finagle-core_2.13:22.7.0! Found 28 potential problems
[error]  * method totalPending()Int in interface com.twitter.finagle.loadbalancer.Balancer does not have a correspondent in current version
[error]    filter with: ProblemFilters.exclude[DirectMissingMethodProblem]("com.twitter.finagle.loadbalancer.Balancer.totalPending")
[error]  * method pending()Int in interface com.twitter.finagle.loadbalancer.LeastLoaded#LeastLoadedNode does not have a correspondent in current version
[error]    filter with: ProblemFilters.exclude[DirectMissingMethodProblem]("com.twitter.finagle.loadbalancer.LeastLoaded#LeastLoadedNode.pending")
[error]  * method totalPending()Int in class com.twitter.finagle.loadbalancer.Metadata does not have a correspondent in current version
[error]    filter with: ProblemFilters.exclude[DirectMissingMethodProblem]("com.twitter.finagle.loadbalancer.Metadata.totalPending")
[error]  * abstract method pending()Int in interface com.twitter.finagle.loadbalancer.NodeT does not have a correspondent in current version
[error]    filter with: ProblemFilters.exclude[DirectMissingMethodProblem]("com.twitter.finagle.loadbalancer.NodeT.pending")
[error]  * class com.twitter.finagle.loadbalancer.PanicMode#ToggledPanicMode does not have a correspondent in current version
[error]    filter with: ProblemFilters.exclude[MissingClassProblem]("com.twitter.finagle.loadbalancer.PanicMode$ToggledPanicMode")
[error]  * class com.twitter.finagle.loadbalancer.PanicModeToggle does not have a correspondent in current version
[error]    filter with: ProblemFilters.exclude[MissingClassProblem]("com.twitter.finagle.loadbalancer.PanicModeToggle")
[error]  * object com.twitter.finagle.loadbalancer.PanicModeToggle does not have a correspondent in current version
[error]    filter with: ProblemFilters.exclude[MissingClassProblem]("com.twitter.finagle.loadbalancer.PanicModeToggle$")
[error]  * method rate()Int in class com.twitter.finagle.loadbalancer.PeakEwma#Metric does not have a correspondent in current version
[error]    filter with: ProblemFilters.exclude[DirectMissingMethodProblem]("com.twitter.finagle.loadbalancer.PeakEwma#Metric.rate")
[error]  * method pending()Int in interface com.twitter.finagle.loadbalancer.PeakEwma#PeakEwmaNode does not have a correspondent in current version
[error]    filter with: ProblemFilters.exclude[DirectMissingMethodProblem]("com.twitter.finagle.loadbalancer.PeakEwma#PeakEwmaNode.pending")
[error]  * method pending()Int in class com.twitter.finagle.loadbalancer.aperture.ApertureLeastLoaded#Node does not have a correspondent in current version
[error]    filter with: ProblemFilters.exclude[DirectMissingMethodProblem]("com.twitter.finagle.loadbalancer.aperture.ApertureLeastLoaded#Node.pending")
[error]  * method pending()Int in class com.twitter.finagle.loadbalancer.aperture.AperturePeakEwma#Node does not have a correspondent in current version
[error]    filter with: ProblemFilters.exclude[DirectMissingMethodProblem]("com.twitter.finagle.loadbalancer.aperture.AperturePeakEwma#Node.pending")
[error]  * static method pick(com.twitter.finagle.loadbalancer.aperture.ProbabilityDistribution,com.twitter.logging.Logger)com.twitter.finagle.loadbalancer.NodeT in class com.twitter.finagle.loadbalancer.aperture.WeightedP2CPick does not have a correspondent in current version
[error]    filter with: ProblemFilters.exclude[DirectMissingMethodProblem]("com.twitter.finagle.loadbalancer.aperture.WeightedP2CPick.pick")
[error]  * method pick(com.twitter.finagle.loadbalancer.aperture.ProbabilityDistribution,com.twitter.logging.Logger)com.twitter.finagle.loadbalancer.NodeT in object com.twitter.finagle.loadbalancer.aperture.WeightedP2CPick does not have a correspondent in current version
[error]    filter with: ProblemFilters.exclude[DirectMissingMethodProblem]("com.twitter.finagle.loadbalancer.aperture.WeightedP2CPick.pick")
[error]  * method pending()Int in class com.twitter.finagle.loadbalancer.p2c.P2CLeastLoaded#Node does not have a correspondent in current version
[error]    filter with: ProblemFilters.exclude[DirectMissingMethodProblem]("com.twitter.finagle.loadbalancer.p2c.P2CLeastLoaded#Node.pending")
[error]  * method pending()Int in class com.twitter.finagle.loadbalancer.p2c.P2CPeakEwma#Node does not have a correspondent in current version
[error]    filter with: ProblemFilters.exclude[DirectMissingMethodProblem]("com.twitter.finagle.loadbalancer.p2c.P2CPeakEwma#Node.pending")
[error]  * method pending()Int in class com.twitter.finagle.loadbalancer.roundrobin.RoundRobinBalancer#Node does not have a correspondent in current version
[error]    filter with: ProblemFilters.exclude[DirectMissingMethodProblem]("com.twitter.finagle.loadbalancer.roundrobin.RoundRobinBalancer#Node.pending")
[error]  * static method apply(Int,Int,com.twitter.finagle.stats.StatsReceiver)java.util.concurrent.ExecutorService in class com.twitter.finagle.offload.OffloadThreadPool does not have a correspondent in current version
[error]    filter with: ProblemFilters.exclude[DirectMissingMethodProblem]("com.twitter.finagle.offload.OffloadThreadPool.apply")
[error]  * method apply(Int,Int,com.twitter.finagle.stats.StatsReceiver)java.util.concurrent.ExecutorService in object com.twitter.finagle.offload.OffloadThreadPool does not have a correspondent in current version
[error]    filter with: ProblemFilters.exclude[DirectMissingMethodProblem]("com.twitter.finagle.offload.OffloadThreadPool.apply")
[error]  * class com.twitter.finagle.offload.OffloadThreadPool#RunsOnNettyThread does not have a correspondent in current version
[error]    filter with: ProblemFilters.exclude[MissingClassProblem]("com.twitter.finagle.offload.OffloadThreadPool$RunsOnNettyThread")
[error]  * class com.twitter.finagle.offload.queueSize does not have a correspondent in current version
[error]    filter with: ProblemFilters.exclude[MissingClassProblem]("com.twitter.finagle.offload.queueSize")
[error]  * object com.twitter.finagle.offload.queueSize does not have a correspondent in current version
[error]    filter with: ProblemFilters.exclude[MissingClassProblem]("com.twitter.finagle.offload.queueSize$")
[error]  * static method module()com.twitter.finagle.Stackable in class com.twitter.finagle.service.DeadlineFilter does not have a correspondent in current version
[error]    filter with: ProblemFilters.exclude[DirectMissingMethodProblem]("com.twitter.finagle.service.DeadlineFilter.module")
[error]  * method this(com.twitter.util.Duration,Double,com.twitter.finagle.stats.StatsReceiver,scala.Function0,scala.Option,Boolean)Unit in class com.twitter.finagle.service.DeadlineFilter does not have a correspondent in current version
[error]    filter with: ProblemFilters.exclude[DirectMissingMethodProblem]("com.twitter.finagle.service.DeadlineFilter.this")
[error]  * method module()com.twitter.finagle.Stackable in object com.twitter.finagle.service.DeadlineFilter does not have a correspondent in current version
[error]    filter with: ProblemFilters.exclude[DirectMissingMethodProblem]("com.twitter.finagle.service.DeadlineFilter.module")
[error]  * abstract method getLocalIdentity()scala.Option in class com.twitter.finagle.ssl.session.SslSessionInfo is present only in current version
[error]    filter with: ProblemFilters.exclude[ReversedMissingMethodProblem]("com.twitter.finagle.ssl.session.SslSessionInfo.getLocalIdentity")
[error]  * abstract method getPeerIdentity()scala.Option in class com.twitter.finagle.ssl.session.SslSessionInfo is present only in current version
[error]    filter with: ProblemFilters.exclude[ReversedMissingMethodProblem]("com.twitter.finagle.ssl.session.SslSessionInfo.getPeerIdentity")
[error]  * method this(com.twitter.finagle.tracing.Tracer,Boolean)Unit in class com.twitter.finagle.tracing.TraceInitializerFilter does not have a correspondent in current version
[error]    filter with: ProblemFilters.exclude[DirectMissingMethodProblem]("com.twitter.finagle.tracing.TraceInitializerFilter.this")
[error]  * abstract method getSampleRate()Float in interface com.twitter.finagle.tracing.Tracer is present only in current version
[error]    filter with: ProblemFilters.exclude[ReversedMissingMethodProblem]("com.twitter.finagle.tracing.Tracer.getSampleRate")
[error] stack trace is suppressed; run last finagle-core / mimaReportBinaryIssues for the full output
[error] (finagle-core / mimaReportBinaryIssues) Failed binary compatibility check against com.twitter:finagle-core_2.13:22.7.0! Found 28 potential problems
[error] Total time: 2 s, completed 02 Mar 2023, 09:04:18
milanvdm commented 1 year ago

@bpholt Im a bit confused on why we run a MiMa on Finagle itself though? Just an extra manual check when bumping versions?

armanbilge commented 1 year ago

I'm kind of surprised there were no breaking changes in this release. Can we double-check that the MiMa "extra projects" are being checked correctly? If so, this lgtm.

My bad, either I forgot when I set it up, or for some reason decided that it would be invoked manually during upgrade PRs like this (presumably because it would be too annoying to add filters). In any case we can change this if you want.

bpholt commented 1 year ago

Im a bit confused on why we run a MiMa on Finagle itself though? Just an extra manual check when bumping versions?

Our team has run into a lot of bincompat issues with Finagle/Twitter Utils over the years, so it's helpful information to have when deciding when to pull in updates. (If there are bincompat issues then you need to update everything using Finagle, including support libraries like this one, etc)

Since it looks like this does pull in binary-breaking changes, we need to make sure the artifact is published with the right version scheme metadata so that e.g. Scala Steward sees this as a major version bump.

milanvdm commented 1 year ago

@bpholt Two things then :)

  1. Should I add the MiMa check to the root project so that it runs on every PR?
  2. Not sure how to set the metadata for the release. Do you have any pointer on how to do this or how it was done in the past?
bpholt commented 1 year ago

I'd lean towards adding it to the build. If it turns out to be too annoying, we can adjust it later.

I think we need ThisBuild / versionScheme := Some("pvp"). (I suggested adding this upstream but I haven't gotten a response: https://github.com/twitter/util/pull/311.)

armanbilge commented 1 year ago

https://github.com/typelevel/catbird/blob/78cd0eccf3e8ffb4a55cf8b27a799bd9c24f694d/build.sbt#L31-L32

bpholt commented 1 year ago

@armanbilge good catch. Ok, I think we're probably in good shape with that part of it!

bpholt commented 1 year ago

Looks good! Once this is merged, we'll release v22.12.0, right?

milanvdm commented 1 year ago

@bpholt Normally yes: ThisBuild / tlBaseVersion := BaseVersion(finagleVersion)