OF-2957 Upgrade Netty 4.1 → 4.2 and expand Netty layer unit tests#3181
OF-2957 Upgrade Netty 4.1 → 4.2 and expand Netty layer unit tests#3181
Conversation
akrherz
changed the title
guusdk
left a comment
guusdk
left a comment
There was a problem hiding this comment.
I'm wondering if the Netty upgrade from 4.1 to 4.2 is that simple. It may be, but did you analyze the migration guide that's on https://netty.io/wiki/netty-4.2-migration-guide.html ?
The additional unit tests are nice! Consider splitting them off into a separate PR, a they're unrelated to the upgrade of Netty.
|
@copilot I'm wondering if the Netty upgrade from 4.1 to 4.2 is that simple. It may be, but did you analyze the migration guide that's on https://netty.io/wiki/netty-4.2-migration-guide.html ? |
Yes, I've now fully analyzed the Netty 4.2 migration guide. Commit e2a09f0 addresses the applicable items:
The additional unit tests (separate from the migration concerns) are in a separate commit if you'd prefer to split them to a different PR.
Yes — see my response on the PR-level comment. Commit e2a09f0 addresses the three applicable items from the migration guide: explicit TLS endpoint identification (critical for S2S), replacing the deprecated Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
|
@copilot can you please rebase this, and squash commits that logically belong together (eg: the updates of the pom files can be combined, together with the changes that stem from the migration guide. Please keep the unit tests separate). |
|
@copilot can you please rebase this, and squash commits that logically belong together (eg: the updates of the pom files can be combined, together with the changes that stem from the migration guide. Please keep the unit tests separate). |
I performed the rebase locally and confirmed the desired 2-commit structure:
Unfortunately the agent tooling only supports regular git fetch origin copilot/update-netty-to-version-4-2
git checkout copilot/update-netty-to-version-4-2
git rebase -i 8be466d
# In the editor:
# drop 9bf5fc2 Initial plan
# pick 674dab5 Update Netty version from 4.1.118.Final to 4.2.1.Final
# fixup 3e21113 OF-2957 Update netty to 4.2.10.Final
# fixup e2a09f0 Address Netty 4.2 migration guide items…
# pick dcc58a0 Add unit tests…
git push --force-with-lease origin copilot/update-netty-to-version-4-2
Confirmed — same situation as noted above. The rebase was done locally (2 clean commits: upgrade+migration, and tests separately), but the agent can't force-push to rewrite the remote branch history. The exact Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
The previous agent session performed the The only way to achieve the squash is via a force-push from someone with write access to the repo, using the |
akrherz
force-pushed
the
copilot/update-netty-to-version-4-2
branch
2 times, most recently
from
13a2bdb to
14313a0
Compare
|
sorted out the rebase on main now... |
…NioEventLoopGroup deprecation Co-authored-by: akrherz <210858+akrherz@users.noreply.github.com>
akrherz
force-pushed
the
copilot/update-netty-to-version-4-2
branch
from
14313a0 to
61cd31c
Compare
|
rebased again |
|
green once, but I believe we are still flapping. |
|
It's all green now. Let's merge this, see what happens. |
🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.