feat: implement oauthbearer token refresh cb setter #546
Conversation
cb-freddysart
force-pushed
the
feat/oauthtoken-cb
branch
from
4aa58ea to
d26ded3
Compare
conf.c
Outdated
|
|
||
| Z_ADDREF_P(&fci.function_name); | ||
|
|
||
| if (conf->cbs.log) { |
There was a problem hiding this comment.
Should be conf->cbs.oauthbearer_token_refresh
conf.c
Outdated
| #ifndef HAS_RD_KAFKA_OAUTHBEARER_TOKEN_REFRESH_CB | ||
| zend_throw_exception_ex(NULL, 0, "This version of rdkafka does not support the OAUTHBEARER sasl mechanism"); | ||
| return; | ||
| #endif |
There was a problem hiding this comment.
Please change this so that the setOauthbearerTokenRefreshCb method does not exist if HAS_RD_KAFKA_OAUTHBEARER_TOKEN_REFRESH_CB is not defined.
(stub files also support #ifdef)
|
Thank you for the speedy review @arnaud-lb ! I'm a bit new to PHP extensions but I believe I've addressed your current round of feedback. Also, sorry for not providing a summary. You beat me to the punch. My goal here is to add support for The other half is adding support for |
|
Nice work, thank you! I'm generally open to adding any method provided by librdkafka. Please do so in a separate PR, as this will simplify reviews (maybe just one PR for the two methods, as they are very related). |
|
hey @arnaud-lb is there any plan to integrate the oauthbearer token refresh also in the high level consumer/producer? |
|
Hey @Rastusik I do not plan to implement this, but I would gladly accept contributions. |
|
Thanks for the info. I think someone is already considering working on it #580 ... I can think about the implementation if I will be forced to use IAM in MSK, but I think the easier way for now would be to just use PLAIN mechanism. |
I'd consider it gladly, but C isn't part of my toolkit unfortunately. However knowing what I know now, I'd tag the ticket as a "feature" rather than a "bug". |
|
@bdurkovic I'm already working on it 6.x...Rastusik:php-rdkafka:oauthbearer_hlc ... but it still needs some polishing |
5 participants
No description provided.