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

[feat][consumer] Support parse broker metadata #276

Merged
merged 1 commit into from
May 30, 2023

Conversation

shoothzj
Copy link
Member

Motivation

pulsar-client-cpp doesn't support parse broker metadata. Which will error whiling connect to enabledbrokerMetada pulsar.
This PR makes pulsar-client-cpp can consume messages well, but haven't exposing index interface yet.

See also

apache/pulsar-client-go#745
https://github.com/apache/pulsar/blob/e38091044c428af002b16110531497e2abc897d2/pulsar-client/src/main/java/org/apache/pulsar/client/impl/ConsumerImpl.java#L1289

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)

@shoothzj
Copy link
Member Author

I'm experiencing some difficulties when running make format and could use some assistance, please.

Traceback (most recent call last):
  File "/Users/akka/code/cpp/pulsar-client-cpp/build-support/run_clang_format.py", line 61, in <module>
    output = subprocess.check_output([CLANG_FORMAT, '-output-replacements-xml']
  File "/Applications/Xcode.app/Contents/Developer/Library/Frameworks/Python3.framework/Versions/3.9/lib/python3.9/subprocess.py", line 424, in check_output
    return run(*popenargs, stdout=PIPE, timeout=timeout, check=True,
  File "/Applications/Xcode.app/Contents/Developer/Library/Frameworks/Python3.framework/Versions/3.9/lib/python3.9/subprocess.py", line 505, in run
    with Popen(*popenargs, **kwargs) as process:
  File "/Applications/Xcode.app/Contents/Developer/Library/Frameworks/Python3.framework/Versions/3.9/lib/python3.9/subprocess.py", line 951, in __init__
    self._execute_child(args, executable, preexec_fn, close_fds,
  File "/Applications/Xcode.app/Contents/Developer/Library/Frameworks/Python3.framework/Versions/3.9/lib/python3.9/subprocess.py", line 1821, in _execute_child
    raise child_exception_type(errno_num, err_msg, err_filename)
FileNotFoundError: [Errno 2] No such file or directory: 'CLANG_FORMAT_BIN-NOTFOUND'
make[3]: *** [CMakeFiles/check-format] Error 1
make[2]: *** [CMakeFiles/check-format.dir/all] Error 2
make[1]: *** [CMakeFiles/check-format.dir/rule] Error 2
make: *** [check-format] Error 2

@shoothzj shoothzj force-pushed the support-parse-broker-metadata branch 2 times, most recently from 5e055a5 to c700dbd Compare May 30, 2023 06:53
@BewareMyPower BewareMyPower added the enhancement New feature or request label May 30, 2023
@BewareMyPower BewareMyPower added this to the 3.3.0 milestone May 30, 2023
@shoothzj shoothzj force-pushed the support-parse-broker-metadata branch 2 times, most recently from e6a57ee to fcebc61 Compare May 30, 2023 08:04
Copy link
Member

@RobertIndie RobertIndie left a comment

Choose a reason for hiding this comment

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

This PR seems more like a fix. Overall looks good to me.
Could you add some tests to verify it?

lib/ClientConnection.cc Outdated Show resolved Hide resolved
lib/ClientConnection.cc Outdated Show resolved Hide resolved
@shoothzj
Copy link
Member Author

@RobertIndie Would it be possible for you to provide me with some examples? I am not familiar with cpp and this repo. I would really appreciate your guidance.

@BewareMyPower
Copy link
Contributor

I think we can easily test this feature by configuring brokerEntryMetadataInterceptors in https://github.com/apache/pulsar-client-cpp/blob/main/test-conf/standalone-ssl.conf. But then the default configs won't be tested.

A better way is to add a test with a custom config like #249. We can add a separated test with a separated Pulsar standalone service.

@shoothzj shoothzj force-pushed the support-parse-broker-metadata branch from fcebc61 to 3f93121 Compare May 30, 2023 11:37
@shoothzj shoothzj force-pushed the support-parse-broker-metadata branch from 3f93121 to ded7b69 Compare May 30, 2023 11:53
tests/brokermetadata/BrokerMetadataTest.cc Outdated Show resolved Hide resolved
tests/brokermetadata/BrokerMetadataTest.cc Outdated Show resolved Hide resolved
tests/brokermetadata/BrokerMetadataTest.cc Outdated Show resolved Hide resolved
lib/ProducerImpl.h Outdated Show resolved Hide resolved
@shoothzj shoothzj force-pushed the support-parse-broker-metadata branch 2 times, most recently from 4969de7 to 3808591 Compare May 30, 2023 12:07
tests/CMakeLists.txt Outdated Show resolved Hide resolved
@shoothzj shoothzj force-pushed the support-parse-broker-metadata branch from 3808591 to a1f5cd0 Compare May 30, 2023 12:30
@shoothzj shoothzj merged commit 0ab478a into main May 30, 2023
@shoothzj shoothzj deleted the support-parse-broker-metadata branch May 30, 2023 13:21
BewareMyPower pushed a commit that referenced this pull request Jun 7, 2023
### 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.
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.

3 participants