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

Client configuration is missing the listenerName parameter #214

Closed
lluiscab opened this issue May 5, 2022 · 7 comments · Fixed by #375
Closed

Client configuration is missing the listenerName parameter #214

lluiscab opened this issue May 5, 2022 · 7 comments · Fixed by #375

Comments

@lluiscab
Copy link

lluiscab commented May 5, 2022

Hi,

Would it be possible to expose the listenerName configuration param from the C++ client?

https://pulsar.apache.org/api/cpp/2.10.0-SNAPSHOT/classpulsar_1_1_client_configuration.html#a2b77a5eb371640f243524a0eed71bc08

@Matt-Esch
Copy link
Contributor

It doesn't appear to be exposed in the c api which is used by this module https://github.com/apache/pulsar/blob/master/pulsar-client-cpp/lib/c/c_ClientConfiguration.cc - I would expect something like pulsar_client_configuration_set_listener_name

I think we would do a lot better to not use the c api but that's a huge refactor.

@lluiscab
Copy link
Author

lluiscab commented May 6, 2022

Yeah :/

I did make a try to implement it myself but I could not find anything exposed by the C api, is there a reason why this module is using the c api when it says in the documentation that it uses the C++ one? The README says: because it uses the [node-addon-api](https://github.com/nodejs/node-addon-api) module to wrap the C++ library. so I'd expect this library to be using the C++ library, not the C one, is that a documentation error then?

I'm trying to connect to a pulsar broker hosted on a k8s cluster from an outside network (Using a public ip address) and not being able to specify the listenerName makes it impossible to connect as the library is picking up the internal (svc.cluster.local) advertised address instead of using the one advertised as external

@Matt-Esch
Copy link
Contributor

I'm not sure what your full use case is but you might want to look at https://github.com/ayeo-flex-org/pulsar-flex - I haven't looked at it properly yet because it doesn't seem complete but it might be easier to extend or complete enough for your use case.

I'm generally very dissatisfied with the c++ library let alone the node binding. I'm currently chasing down an issue with a race condition on close that causes a segfault. That should stir some level of caution around using this library and it clearly needs a lot of work. I think the reason for using the c api is that it's more stable between versions, so that also means not having the latest features. I did briefly look at using the C++ api because there are features missing for the async interface (that and the memory management is exceptionally painful).

@roryschadler
Copy link
Contributor

Started a PR to expose this in the C API here: apache/pulsar-client-cpp#370

Once that's merged/released, I'd be happy to do the same here.

@roryschadler
Copy link
Contributor

@Matt-Esch unrelated to this issue, but I'm now also dealing with a segfault, and it seems like it's around the .close methods, in my case combined with a listener callback: #374. Is this similar to the issue you faced, do you remember? Did you ever find a solution?

@lluiscab
Copy link
Author

lluiscab commented Apr 4, 2024

Hey @roryschadler, given that v3.5.0 of the cpp client has been published containing apache/pulsar-client-cpp#370, is there any ETA on resolving this issue?

I've never developed any node project that used bindings so I'm not sure I could open a PR myself, specially given that there are new methods that need to be "bound" and exposed by the node client.

@roryschadler
Copy link
Contributor

roryschadler commented Apr 4, 2024

@lluiscab I missed the release, thanks for the heads up! I have a draft PR here (been using this change locally for a while), need to add tests and then it'll be ready for review. #375

@shibd shibd closed this as completed in #375 Apr 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants