Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update r2dbc dependencies #60

Merged
merged 1 commit into from
Sep 13, 2023

Conversation

mdedetrich
Copy link
Contributor

The context for this is an upcoming PR to add license-report and updating these dependencies will make things slightly easier in the long run.

The dependencies being updated are bincompat so there shouldn't be any issues

@pjfanning
Copy link
Contributor

Could you provide some of the context? Generally, for 1.0.0 releases, we only upgrade if there are CVEs to worry about.

@mdedetrich
Copy link
Contributor Author

mdedetrich commented Sep 13, 2023

Could you provide some of the context? Generally, for 1.0.0 releases, we only upgrade if there are CVEs to worry about.

So basically I was trying to add sbt-license-report which led me down to a massive rabbit hole, the tl;dr is that I can still add sbt-license-report but in doing so I have to use this workaround in Dependencies.scala

    // This is here because sbt's ivy resolver doesn't properly support packaging.type when
    // resolving via sbt-license-report, see https://github.com/sbt/sbt/issues/3618
    val r2dbcPostgres = Seq(
      ("org.postgresql" % "r2dbc-postgresql" % "0.9.1.RELEASE").excludeAll(
        "io.netty.incubator", "netty-incubator-codec-native-quic"),
      ("io.netty.incubator" % "netty-incubator-codec-native-quic" % "0.0.26.Final")
        .artifacts(Artifact("netty-incubator-codec-native-quic",
          url("https://repo1.maven.org/maven2/io/netty/incubator/netty-incubator-codec-native-quic/0.0.26.Final/netty-incubator-codec-native-quic-0.0.26.Final.test"))))

The workaround has zero impact on the binary/dependency resolution, but as you can see its somewhat hacky because i have to override the artifact with a manual URI. Given that the URI is tied to a specific version of netty-incubator-codec-native-quic which is inherited from r2dbc-postgresql, I wanted to just update r2dbc-postgresql to the latest bincompat versions since this workaround is brittle (i.e. if you update r2dbc-postgresql and not netty-incubator-codec-native-quic then you incorrectly overrode the dependencies being resolved).

Hence why I just want to update to the latest bincompat version since that reduces the risk of someone updating the dependencies and doing it incorrectly (i.e. possibly scala-steward in the future) to mitigate this risk.

See sbt/sbt-license-report#87 and sbt/sbt-license-report#86 for additional context

Copy link
Contributor

@pjfanning pjfanning left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@pjfanning pjfanning added the release notes should be mentioned in release notes label Sep 13, 2023
@mdedetrich mdedetrich merged commit 29b79e5 into apache:main Sep 13, 2023
19 checks passed
@mdedetrich mdedetrich deleted the update-r2dbc-dependencies branch September 13, 2023 16:08
@mdedetrich mdedetrich mentioned this pull request Sep 13, 2023
@pjfanning pjfanning added this to the v1.0.0 milestone Dec 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release notes should be mentioned in release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants