Replacing InboundMessage with NativeInboundMessage for deprecation #13126
Conversation
VachaShah
requested review from
peternied,
abbashus,
adnapibar,
anasalkouz,
andrross,
Bukhtawar,
CEHENKLE,
dblock,
dbwiddis,
dreamer-89,
gbbafna,
kotwanikunal,
mch2,
msfroh,
nknize,
owaiskazi19,
reta,
Rishikesh1159,
ryanbogan,
sachinpkale,
saratvemulapalli,
shwetathareja,
sohami and
tlfeng
as code owners
Compatibility status:Checks if related components are compatible with change 3d4ab18 Incompatible componentsSkipped componentsCompatible componentsCompatible components: [https://github.com/opensearch-project/custom-codecs.git, https://github.com/opensearch-project/asynchronous-search.git, https://github.com/opensearch-project/neural-search.git, https://github.com/opensearch-project/flow-framework.git, https://github.com/opensearch-project/cross-cluster-replication.git, https://github.com/opensearch-project/job-scheduler.git, https://github.com/opensearch-project/reporting.git, https://github.com/opensearch-project/security-analytics.git, https://github.com/opensearch-project/geospatial.git, https://github.com/opensearch-project/opensearch-oci-object-storage.git, https://github.com/opensearch-project/notifications.git, https://github.com/opensearch-project/common-utils.git, https://github.com/opensearch-project/k-nn.git, https://github.com/opensearch-project/observability.git, https://github.com/opensearch-project/anomaly-detection.git, https://github.com/opensearch-project/ml-commons.git, https://github.com/opensearch-project/performance-analyzer-rca.git, https://github.com/opensearch-project/alerting.git, https://github.com/opensearch-project/index-management.git, https://github.com/opensearch-project/sql.git, https://github.com/opensearch-project/security.git, https://github.com/opensearch-project/performance-analyzer.git] |
|
❌ Gradle check result for 38e668c: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
|
@VachaShah this is for 3.0 only, right? PS: I think we'll have hard time backporting further changes if we take this path now |
Yeah this is only for 3.0 since |
Test Result (1 failure / +1) |
|
Looks like the BWC tests started failing after removing the |
Signed-off-by: Vacha Shah <[email protected]>
Signed-off-by: Vacha Shah <[email protected]>
VachaShah
force-pushed
the
inboundmessage-replace
branch
from
3d4ab18 to
adccc66
Compare
|
❌ Gradle check result for adccc66: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
Test Result (1 failure / +1) |
|
@reta Good to merge the breaking change? |
Ah ... @dblock could you please hold on? We've been recently fighting with an issue (along with @peternied ) that merging breaking change in one pull request impacts the checks for other pull requests. There is nothing wrong with breaking change in major release, but we have a flaw in our checks currently. |
|
@dblock we should be cleared out, the check is disabled on |
|
Note on the failing |
…ation (opensearch-project#13126)" This reverts commit f5c3ef9.
…ation (opensearch-project#13126)" This reverts commit f5c3ef9.
…ation (opensearch-project#13126)" This reverts commit f5c3ef9. Signed-off-by: Andrew Ross <[email protected]>
…ation (opensearch-project#13126)" This reverts commit f5c3ef9. Signed-off-by: Andrew Ross <[email protected]>
…ation (opensearch-project#13126)" This reverts commit f5c3ef9. Signed-off-by: Andrew Ross <[email protected]>
* Revert "Replacing InboundMessage with NativeInboundMessage for deprecation (#13126)" This reverts commit f5c3ef9. Signed-off-by: Andrew Ross <[email protected]> * Change abstraction point for transport protocol The previous implementation had a transport switch point in InboundPipeline when the bytes were initially pulled off the wire. There was no implementation for any other protocol as the `canHandleBytes` method was hardcoded to return true. I believe this is the wrong point to switch on the protocol. This change makes NativeInboundBytesHandler protocol agnostic beyond the header. With this change, a complete message is parsed from the stream of bytes, with the header schema being unchanged from what exists today. The protocol switch point will now be at `InboundHandler::inboundMessage`. The header will indicate what protocol was used to serialize the the non-header bytes of the message and then invoke the appropriate handler based on that field. Signed-off-by: Andrew Ross <[email protected]> --------- Signed-off-by: Andrew Ross <[email protected]>
…5432) * Revert "Replacing InboundMessage with NativeInboundMessage for deprecation (opensearch-project#13126)" This reverts commit f5c3ef9. Signed-off-by: Andrew Ross <[email protected]> * Change abstraction point for transport protocol The previous implementation had a transport switch point in InboundPipeline when the bytes were initially pulled off the wire. There was no implementation for any other protocol as the `canHandleBytes` method was hardcoded to return true. I believe this is the wrong point to switch on the protocol. This change makes NativeInboundBytesHandler protocol agnostic beyond the header. With this change, a complete message is parsed from the stream of bytes, with the header schema being unchanged from what exists today. The protocol switch point will now be at `InboundHandler::inboundMessage`. The header will indicate what protocol was used to serialize the the non-header bytes of the message and then invoke the appropriate handler based on that field. Signed-off-by: Andrew Ross <[email protected]> --------- Signed-off-by: Andrew Ross <[email protected]>
…5432) * Revert "Replacing InboundMessage with NativeInboundMessage for deprecation (opensearch-project#13126)" This reverts commit f5c3ef9. Signed-off-by: Andrew Ross <[email protected]> * Change abstraction point for transport protocol The previous implementation had a transport switch point in InboundPipeline when the bytes were initially pulled off the wire. There was no implementation for any other protocol as the `canHandleBytes` method was hardcoded to return true. I believe this is the wrong point to switch on the protocol. This change makes NativeInboundBytesHandler protocol agnostic beyond the header. With this change, a complete message is parsed from the stream of bytes, with the header schema being unchanged from what exists today. The protocol switch point will now be at `InboundHandler::inboundMessage`. The header will indicate what protocol was used to serialize the the non-header bytes of the message and then invoke the appropriate handler based on that field. Signed-off-by: Andrew Ross <[email protected]> --------- Signed-off-by: Andrew Ross <[email protected]>
…5432) * Revert "Replacing InboundMessage with NativeInboundMessage for deprecation (opensearch-project#13126)" This reverts commit f5c3ef9. Signed-off-by: Andrew Ross <[email protected]> * Change abstraction point for transport protocol The previous implementation had a transport switch point in InboundPipeline when the bytes were initially pulled off the wire. There was no implementation for any other protocol as the `canHandleBytes` method was hardcoded to return true. I believe this is the wrong point to switch on the protocol. This change makes NativeInboundBytesHandler protocol agnostic beyond the header. With this change, a complete message is parsed from the stream of bytes, with the header schema being unchanged from what exists today. The protocol switch point will now be at `InboundHandler::inboundMessage`. The header will indicate what protocol was used to serialize the the non-header bytes of the message and then invoke the appropriate handler based on that field. Signed-off-by: Andrew Ross <[email protected]> --------- Signed-off-by: Andrew Ross <[email protected]>
…5432) * Revert "Replacing InboundMessage with NativeInboundMessage for deprecation (opensearch-project#13126)" This reverts commit f5c3ef9. Signed-off-by: Andrew Ross <[email protected]> * Change abstraction point for transport protocol The previous implementation had a transport switch point in InboundPipeline when the bytes were initially pulled off the wire. There was no implementation for any other protocol as the `canHandleBytes` method was hardcoded to return true. I believe this is the wrong point to switch on the protocol. This change makes NativeInboundBytesHandler protocol agnostic beyond the header. With this change, a complete message is parsed from the stream of bytes, with the header schema being unchanged from what exists today. The protocol switch point will now be at `InboundHandler::inboundMessage`. The header will indicate what protocol was used to serialize the the non-header bytes of the message and then invoke the appropriate handler based on that field. Signed-off-by: Andrew Ross <[email protected]> --------- Signed-off-by: Andrew Ross <[email protected]>
Description
InboundMessagewas deprecated in #12967 and replaced withNativeInboundMessage. Replacing instances in code for the same.Related Issues
Resolves #[Issue number to be closed when this PR is merged]
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.