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 airbase to 148 #19607

Merged
merged 1 commit into from
Nov 7, 2023
Merged

Update airbase to 148 #19607

merged 1 commit into from
Nov 7, 2023

Conversation

wendigo
Copy link
Contributor

@wendigo wendigo commented Nov 1, 2023

Fixes incorrect junit/surefire integration that isn't working with the latest surefire plugin.

junit-jupiter-engine should be a module dependency, rather then surefire plugin dependency.

@cla-bot cla-bot bot added the cla-signed label Nov 1, 2023
@wendigo wendigo force-pushed the serafin/airbase-148 branch 3 times, most recently from 2d73453 to ce80fb7 Compare November 4, 2023 15:59
Fixes incorrect junit/surefire integration
that isn't working with the latest surefire plugin.

junit-jupiter-engine should be a module dependency,
rather then surefire plugin dependency.
@wendigo wendigo requested a review from martint November 4, 2023 18:20
@github-actions github-actions bot added jdbc Relates to Trino JDBC driver tests:hive hudi Hudi connector delta-lake Delta Lake connector hive Hive connector bigquery BigQuery connector labels Nov 4, 2023
@wendigo wendigo requested a review from electrum November 4, 2023 18:22
@martint
Copy link
Member

martint commented Nov 4, 2023

@wendigo
Copy link
Contributor Author

wendigo commented Nov 4, 2023

@martint but it doesn't work anymore :( We had it mixed anyway. Some modules were following method in this PR, some were adding engine to the surefire deps. Now it seems to work properly and all transitive dependencies that are needed are resolved (this wasn't the case when the jupiter-engine was in the surefire deps).

@wendigo
Copy link
Contributor Author

wendigo commented Nov 5, 2023

@martint PTAL

@martint
Copy link
Member

martint commented Nov 6, 2023

But do we know why it starts failing with the surefire plugin upgrade? That seems like a regression in the plugin, and would suggest we shouldn't update until that bug is fixed.

@wendigo
Copy link
Contributor Author

wendigo commented Nov 6, 2023

@martint I guess that this is related to apache/maven-surefire#617. Anyway I personally think that current behaviour is the correct one. Not the previous one

@wendigo
Copy link
Contributor Author

wendigo commented Nov 6, 2023

@martint I'd like to move forward with this change as it contains some needed dependency upgrades.

@wendigo
Copy link
Contributor Author

wendigo commented Nov 7, 2023

Ping @martint

@martint
Copy link
Member

martint commented Nov 7, 2023

Anyway I personally think that current behaviour is the correct one. Not the previous one

How so? It would be preferable to be able to specify the dependencies in the plugin spec so that they could potentially be applied to the whole project eventually by configuring it in the root pom instead.

I'm fine with doing this for now if necessary to update the other dependencies you mentioned, but as soon as the underlying bug in surefire is fixed, we should change it back.

@wendigo
Copy link
Contributor Author

wendigo commented Nov 7, 2023

@martint i'm ok with that

@wendigo wendigo merged commit c0d03b6 into master Nov 7, 2023
96 checks passed
@wendigo wendigo deleted the serafin/airbase-148 branch November 7, 2023 20:08
@github-actions github-actions bot added this to the 433 milestone Nov 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bigquery BigQuery connector cla-signed delta-lake Delta Lake connector hive Hive connector hudi Hudi connector jdbc Relates to Trino JDBC driver
Development

Successfully merging this pull request may close these issues.

2 participants