Fix AbstractManagedChannelImplBuilder#maxInboundMessageSize(int) ABI #8607
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
sergiitk
force-pushed
the
builders-abi-maxinboundmessagesize
branch
2 times, most recently
from
d23206a to
a3e3051
Compare
In refactoring described in grpc#7211, the implementation of #maxInboundMessageSize(int) (and its corresponding field) were pulled down from internal AbstractManagedChannelImplBuilder to concrete classes that actually enforce this setting. For the same reason, it wasn't ported to ManagedChannelImplBuilder (the #delegate()). Then AbstractManagedChannelImplBuilder was brought back to fix ABI backward compatibility, and temporarily turned into a ForwardingChannelBuilder, ref PR grpc#7564. Eventually it will be deleted, after a period with "bridge" ABI solution introduced in grpc#7834. However, restoring AbstractManagedChannelImplBuilder unintentionally made ABI of pre-refactoring builds expect it to be a method of AbstractManagedChannelImplBuilder, and not concrete classes, ref grpc#8313. The end goal is to keep #maxInboundMessageSize(int) only in concrete classes that enforce it. To fix method's ABI, we temporary reintroduce it to the original layer it was removed from: AbstractManagedChannelImplBuilder. This class' only intention is to provide short-term ABI compatibility. Once we move forward with dropping the ABI, both fixes are no longer necessary, and both will perish with removing AbstractManagedChannelImplBuilder.
sergiitk
force-pushed
the
builders-abi-maxinboundmessagesize
branch
from
a3e3051 to
06e81fa
Compare
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In refactoring described in #7211, the implementation of #maxInboundMessageSize(int)
(and its corresponding field) was pulled down from internal AbstractManagedChannelImplBuilder
to concrete classes that actually enforce this setting. For the same reason, it wasn't ported
to ManagedChannelImplBuilder (the #delegate()).
Then AbstractManagedChannelImplBuilder was brought back to fix ABI backward compatibility,
and temporarily turned into a ForwardingChannelBuilder, ref PR #7564. Eventually it will
be deleted, after a period with "bridge" ABI solution introduced in #7834.
However, restoring AbstractManagedChannelImplBuilder unintentionally made ABI of
#maxInboundMessageSize(int) implemented by the concrete classes backward incompatible:
pre-refactoring builds expect it to be a method of AbstractManagedChannelImplBuilder,
and not concrete classes, ref #8313.
The end goal is to keep #maxInboundMessageSize(int) only in concrete classes that enforce it.
To fix method's ABI, we temporary reintroduce it to the original layer it was removed from:
AbstractManagedChannelImplBuilder. This class' only intention is to provide short-term
ABI compatibility. Once we move forward with dropping the ABI, both fixes are no longer
necessary, and both will perish with removing AbstractManagedChannelImplBuilder.
Tested manually by reproducing #8313, then confirming this PR fixes it.
This is an alternative to #8597
Fixes #8313