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

Upgrade Undertow to 2.2.24.Final #2580

Merged
merged 6 commits into from Apr 3, 2023
Merged

Conversation

carterkozak
Copy link
Contributor

==COMMIT_MSG==
Upgrade Undertow to 2.2.24.Final
==COMMIT_MSG==

Possible downsides?

Oddness due to interactions between the undertow javax/jakarta migration scheme and the way gradle-consistent-versions works. We test with an older undertow-servlet release, which is dangerous, but a reasonable risk in test-only code. Previously we sat on an old release.

# updated to jakarta only. This should NEVER be done in production code, it is only used
# here to enable existing test coverage.
io.undertow:undertow-servlet-jakarta = 2.2.20.Final
# dependency-upgrader:ON
Copy link
Contributor

Choose a reason for hiding this comment

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

@carterkozak should we also bump com.palantir.conjure.java to 7.5.0 in this release? I'm also not sure why we have separate pin for com.palantir.conjure.java:conjure-lib = 7.4.0

com.palantir.conjure.java:* = 7.5.0

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good call on the unnecessary conjure-lib, I'm not sure where that came from. I don't think conjure-java 7.5.0 is published yet

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah, I got 404s locally for com.palantir.conjure.java:conjure-lib:7.5.0 just a second ago. Will +1 and defer to you if you want to wait for the publish to complete before merging this & cutting CJR release as well

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removing the duplication causes consistent CI failures. I'm going to undo that piece and investigate later on.

# updated to jakarta only. This should NEVER be done in production code, it is only used
# here to enable existing test coverage.
io.undertow:undertow-servlet-jakarta = 2.2.20.Final
# dependency-upgrader:ON
Copy link
Contributor

Choose a reason for hiding this comment

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

yeah, I got 404s locally for com.palantir.conjure.java:conjure-lib:7.5.0 just a second ago. Will +1 and defer to you if you want to wait for the publish to complete before merging this & cutting CJR release as well

versions.props Outdated
com.palantir.conjure.java:* = 7.4.0
com.palantir.conjure.java:conjure-lib = 7.4.0
com.palantir.conjure.java:* = 7.5.0
om.palantir.conjure.java:conjure-lib = 7.5.0
Copy link
Contributor

Choose a reason for hiding this comment

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

sorry about the wild goose chase on this, something seems odd with :conjure-java-client-verifier requiring this pin, but fine leaving this extra pin here

Suggested change
om.palantir.conjure.java:conjure-lib = 7.5.0
com.palantir.conjure.java:conjure-lib = 7.5.0

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍 I'll follow up on this one separately

Copy link
Contributor

@schlosna schlosna Apr 3, 2023

Choose a reason for hiding this comment

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

yeah, not urgent, just seemed odd when looking through pins, but here be 🐉

@bulldozer-bot bulldozer-bot bot merged commit d00a69b into develop Apr 3, 2023
@bulldozer-bot bulldozer-bot bot deleted the ckozak/latest-undertow branch April 3, 2023 16:09
@svc-autorelease
Copy link
Collaborator

Released 7.45.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants