Skip to content

[feat][Message] Add getIndex method on Message #277

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

Merged
merged 1 commit into from
Jun 7, 2023

Conversation

hezhangjian
Copy link
Member

Motivation

After #276 , We already can consume messages contains index well. Now we can add a method allow user to getIndex.
Index is an optional brokerMetadata. If the index not exists, we will return -1

Verifying this change

  • Add the assertions to assert the index not equals -1.

Documentation

  • doc-required
    (Your PR needs to update docs and you will update later)

  • doc-not-needed
    (Please explain why)

  • doc
    (Your PR contains doc changes)

  • doc-complete
    (Docs have been already added)

@hezhangjian hezhangjian force-pushed the add-get-index-method branch 2 times, most recently from 54a1dc8 to 1e15921 Compare June 1, 2023 01:29
@hezhangjian
Copy link
Member Author

@BewareMyPower PTAL

@BewareMyPower BewareMyPower added this to the 3.3.0 milestone Jun 1, 2023
@BewareMyPower BewareMyPower added the enhancement New feature or request label Jun 1, 2023
@hezhangjian hezhangjian force-pushed the add-get-index-method branch 4 times, most recently from eebba7f to 32d22cc Compare June 2, 2023 00:02
Copy link
Contributor

@BewareMyPower BewareMyPower left a comment

Choose a reason for hiding this comment

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

image

You can see the BrokerEntryMetadata was not added when messages were from the C++ producer.

From the server side, you can see:

if (cnx.getRemoteEndpointProtocolVersion() < ProtocolVersion.v18.getValue()
        || !cnx.supportBrokerMetadata()
        || !cnx.getBrokerService().getPulsar().getConfig()
        .isExposingBrokerEntryMetadataToClientEnabled()) {
    Commands.skipBrokerEntryMetadataIfExist(metadataAndPayload);
}

The client needs to set the feature flags in newConnect, just like the Java client does:

    private static void setFeatureFlags(FeatureFlags flags) {
        flags.setSupportsAuthRefresh(true);
        flags.setSupportsBrokerEntryMetadata(true);
        flags.setSupportsPartialProducer(true);
    }

@hezhangjian hezhangjian force-pushed the add-get-index-method branch 4 times, most recently from 59efecc to a449b62 Compare June 6, 2023 04:52
@BewareMyPower BewareMyPower dismissed their stale review June 6, 2023 08:17

Code has changed

@hezhangjian hezhangjian force-pushed the add-get-index-method branch 2 times, most recently from dcdd19c to fe96af0 Compare June 6, 2023 12:23
@hezhangjian hezhangjian force-pushed the add-get-index-method branch 2 times, most recently from 3a2dff7 to c1bbcb1 Compare June 6, 2023 16:00
@hezhangjian hezhangjian force-pushed the add-get-index-method branch from c1bbcb1 to bf6d2fd Compare June 7, 2023 01:02
@hezhangjian
Copy link
Member Author

ping @BewareMyPower

@BewareMyPower BewareMyPower merged commit d03e46a into main Jun 7, 2023
@BewareMyPower BewareMyPower deleted the add-get-index-method branch June 7, 2023 03:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants