Skip to content

Update org.yaml:snakeyaml dependency to 1.28#12091

Merged
hashhar merged 1 commit intotrinodb:masterfrom
leveyja:leveyja/trino-snakeyaml-dep
Apr 25, 2022
Merged

Update org.yaml:snakeyaml dependency to 1.28#12091
hashhar merged 1 commit intotrinodb:masterfrom
leveyja:leveyja/trino-snakeyaml-dep

Conversation

@leveyja
Copy link
Member

@leveyja leveyja commented Apr 22, 2022

Description

trino-product-tests-launcher requires org.yaml:snakeyaml:1.28, while Trino's root pom.xml specifies 1.26.
The snakeyaml dependency comes from jackson-dataformat-yaml:2.13.1, which is the version specified by io.airlift:airbase:123
(see https://github.com/airlift/airbase/blob/295990f5fc4637b89f98ab11a0e796ce3344e7e0/airbase/pom.xml#L1125 )

This PR updates Trino's pom.xml to specify the same version of com.yaml:snakeyaml:1.28 that is specified via airbase:123.

This also allows us to remove exclusions on enforcer dependency upper range rules (see trino-cassandra pom.xml).

Is this change a fix, improvement, new feature, refactoring, or other?

Improvement

Is this a change to the core query engine, a connector, client library, or the SPI interfaces? (be specific)

Dependency version upgrade

How would you describe this change to a non-technical end user or system administrator?

Upgrading 3rd party libraries to latest common version

Related issues, pull requests, and links

Documentation

(*) No documentation is needed.
( ) Sufficient documentation is included in this PR.
( ) Documentation PR is available with #prnumber.
( ) Documentation issue #issuenumber is filed, and can be handled later.

Release notes

(*) No release notes entries required.
( ) Release notes entries required with the following suggested text:

# Section
* Fix some things. ({issue}`issuenumber`)

@cla-bot cla-bot bot added the cla-signed label Apr 22, 2022
@leveyja leveyja requested review from findepi and nineinchnick April 22, 2022 05:59
@leveyja leveyja force-pushed the leveyja/trino-snakeyaml-dep branch from e9657a2 to dd11ede Compare April 22, 2022 06:00
@leveyja leveyja changed the title Upgrade org.snakeyaml:snakeyaml dependency to 1.30 Upgrade org.yaml:snakeyaml dependency to 1.30 Apr 22, 2022
Copy link
Member

@hashhar hashhar left a comment

Choose a reason for hiding this comment

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

Thanks. Can you verify whether there are other places it is being pulled in but not causing issues from the mvn dependency:tree output?

Please add some justification to commit message or reword to something like

Ensure same version of snakeyaml is used across modules

Versions must be consistent to avoid having to add exclusions because org.yaml:snakeyaml is a transitive dependency of many dependencies.

@leveyja
Copy link
Member Author

leveyja commented Apr 22, 2022

Thanks @hashhar - I've updated the commit message:

    Ensure single version of snakeyaml is used across modules

    Versions must be consistent to avoid having to add exclusions to enforcer rules.
    org.yaml:snakeyaml is a transitive dependency of trino-product-tests-launcher.

Here's a breakdown of dependencies before/after the dependency version change:
Master (379-SNAPSHOT): ( ./mvnw dependency:tree -Dincludes='org.yaml:snakeyaml:*' | tee yaml-master.log )

[INFO] --- maven-dependency-plugin:3.3.0:tree (default-cli) @ trino-testing-services ---
[INFO] io.trino:trino-testing-services:jar:379-SNAPSHOT
[INFO] \- io.trino.tempto:tempto-core:jar:187:test
[INFO]    \- org.yaml:snakeyaml:jar:1.26:test
[...]
[INFO] io.trino:trino-benchto-benchmarks:jar:379-SNAPSHOT
[INFO] \- io.trino.benchto:benchto-driver:jar:0.17:compile
[INFO]    \- org.yaml:snakeyaml:jar:1.26:compile
[...]
[INFO] --- maven-dependency-plugin:3.3.0:tree (default-cli) @ trino-product-tests-launcher ---
[INFO] io.trino:trino-product-tests-launcher:jar:379-SNAPSHOT
[INFO] \- com.fasterxml.jackson.dataformat:jackson-dataformat-yaml:jar:2.13.1:compile
[INFO]    \- org.yaml:snakeyaml:jar:1.28:compile

Here are the snakeyaml depenencies "1.30" version (upgrade) applied:

[INFO] --- maven-dependency-plugin:3.3.0:tree (default-cli) @ trino-testing-services ---
[INFO] io.trino:trino-testing-services:jar:379-SNAPSHOT
[INFO] \- io.trino.tempto:tempto-core:jar:187:test
[INFO]    \- org.yaml:snakeyaml:jar:1.30:test
[...]
[INFO] --- maven-dependency-plugin:3.3.0:tree (default-cli) @ trino-benchto-benchmarks ---
[INFO] io.trino:trino-benchto-benchmarks:jar:379-SNAPSHOT
[INFO] \- io.trino.benchto:benchto-driver:jar:0.17:compile
[INFO]    \- org.yaml:snakeyaml:jar:1.30:compile
[...]
[INFO] io.trino:trino-product-tests:jar:379-SNAPSHOT
[INFO] \- io.trino.tempto:tempto-core:jar:187:compile
[INFO]    \- org.yaml:snakeyaml:jar:1.30:compile
[...]
[INFO] io.trino:trino-product-tests-launcher:jar:379-SNAPSHOT
[INFO] \- com.fasterxml.jackson.dataformat:jackson-dataformat-yaml:jar:2.13.1:compile
[INFO]    \- org.yaml:snakeyaml:jar:1.30:compile

(trino-cassandra doesn't appear to have a dependency on snakeyaml any more, so we could remove that enforcer exclusion regardless).
Are there any other possible issues the upgrade could cause that I should investigate?

Update: Here are the dependencies w/this patch/1.28 set as the common version (in order to match airbase w/out upgrading)

[INFO] io.trino:trino-testing-services:jar:379-SNAPSHOT
[INFO] \- io.trino.tempto:tempto-core:jar:187:test
[INFO]    \- org.yaml:snakeyaml:jar:1.28:test
[...]
[INFO] io.trino:trino-benchto-benchmarks:jar:379-SNAPSHOT
[INFO] \- io.trino.benchto:benchto-driver:jar:0.17:compile
[INFO]    \- org.yaml:snakeyaml:jar:1.28:compile
[...]
[INFO] io.trino:trino-product-tests:jar:379-SNAPSHOT
[INFO] \- io.trino.tempto:tempto-core:jar:187:compile
[INFO]    \- org.yaml:snakeyaml:jar:1.28:compile
[...]
[INFO] io.trino:trino-product-tests-launcher:jar:379-SNAPSHOT
[INFO] \- com.fasterxml.jackson.dataformat:jackson-dataformat-yaml:jar:2.13.1:compile
[INFO]    \- org.yaml:snakeyaml:jar:1.28:compile
[...]

@leveyja
Copy link
Member Author

leveyja commented Apr 22, 2022

I've checked the versions in io.airlift:airbase:123

<dep.jackson.version>2.13.1</dep.jackson.version>
[...]
            <dependency>
                <groupId>com.fasterxml.jackson.dataformat</groupId>
                <artifactId>jackson-dataformat-yaml</artifactId>
                <version>${dep.jackson.version}</version>
            </dependency>

So com.yaml:snakeyaml:1.28 is the version referenced transitively from airlift's jackson-dataformat-yaml:2.13.1 bom.

I'm going to update the PR to upgrade to 1.28, just to keep the changes to a minimum / not let trino get ahead of airbase's bom version of snakeyaml.

@leveyja leveyja force-pushed the leveyja/trino-snakeyaml-dep branch from dd11ede to 513bfba Compare April 22, 2022 07:49
@leveyja leveyja changed the title Upgrade org.yaml:snakeyaml dependency to 1.30 Upgrade org.yaml:snakeyaml dependency to 1.28 Apr 22, 2022
@leveyja leveyja changed the title Upgrade org.yaml:snakeyaml dependency to 1.28 Update org.yaml:snakeyaml dependency to 1.28 Apr 22, 2022
@leveyja leveyja requested a review from hashhar April 22, 2022 07:55
Copy link
Member

@hashhar hashhar left a comment

Choose a reason for hiding this comment

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

transitive dependency of jackson-dataformat-yaml -> transitive dependency of jackson-dataformat-yaml from Airbase.

Thanks for noticing the inclusion from Airbase. 1.28 makes more sense with that context.

@leveyja leveyja force-pushed the leveyja/trino-snakeyaml-dep branch from 513bfba to 4c916cd Compare April 22, 2022 09:22
Versions must be consistent to avoid having to add exclusions to enforcer rules.
org.yaml:snakeyaml:1.28 is a transitive dependency of jackson-dataformat-yaml
from Airbase.
@leveyja leveyja force-pushed the leveyja/trino-snakeyaml-dep branch from 4c916cd to b2c74df Compare April 22, 2022 09:22
@hashhar hashhar merged commit 8cfe0a7 into trinodb:master Apr 25, 2022
@github-actions github-actions bot added this to the 379 milestone Apr 25, 2022
@leveyja leveyja deleted the leveyja/trino-snakeyaml-dep branch April 25, 2022 08:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

3 participants