Add getControllerId #554
Add getControllerId #554
Conversation
qkdreyer
force-pushed
the
controllerid
branch
5 times, most recently
from
d66f9e3 to
1f87298
Compare
|
@arnaud-lb not sure why CI is failing; I've tried upgrading the docker images to their latest version in 1f87298 but it looks like that's not compatible with v0.11.6, wdyt? |
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
| docker run -d --network kafka_network --name zookeeper wurstmeister/zookeeper:3.4.6 | ||
| docker pull wurstmeister/kafka:2.13-2.6.0 | ||
| docker run -d -p 9092:9092 --network kafka_network -e "KAFKA_AUTO_CREATE_TOPICS_ENABLE=true" -e "KAFKA_CREATE_TOPICS=test-topic:1:1:compact" -e "KAFKA_ADVERTISED_HOST_NAME=kafka" -e "KAFKA_ZOOKEEPER_CONNECT=zookeeper:2181" -e "KAFKA_ADVERTISED_PORT=9092" --name kafka wurstmeister/kafka:2.13-2.6.0 | ||
| docker pull wurstmeister/zookeeper:latest |
There was a problem hiding this comment.
Please use a fixed version here, otherwise the CI may break independently of changes made in this repos
There was a problem hiding this comment.
the thing is wurstmeister/zookeeper only exists with 3.4.6 and latest tags, and 3.4.6 tag seems broken for 2 months
kafka_consumer.c
Outdated
| Returns the current ControllerId (controller broker id) as reported in broker metadata */ | ||
| PHP_METHOD(RdKafka_KafkaConsumer, getControllerId) | ||
| { | ||
| kafka_object *intern; |
There was a problem hiding this comment.
| kafka_object *intern; | |
| object_intern *intern; |
tests/controller_id.phpt
Outdated
| $conf->set('metadata.broker.list', getenv('TEST_KAFKA_BROKERS')); | ||
| $conf->set('group.id', 'test'); | ||
|
|
||
| echo (new RdKafka\KafkaConsumer($conf))->getControllerId(0) . \PHP_EOL; |
There was a problem hiding this comment.
Setting a timeout > 0 would make this test more robust. I suggest a timeout of 10*1000 as in getMetadata() tests.
| @@ -528,6 +528,28 @@ PHP_METHOD(RdKafka_KafkaConsumer, getMetadata) | |||
| } | |||
| /* }}} */ | |||
|
|
|||
| #ifdef HAS_RD_KAFKA_CONTROLLERID | |||
There was a problem hiding this comment.
rd_kafka_controllerid appears to be broken in librdkafka 0.11.x. I suggest not compiling this method on this version:
| #ifdef HAS_RD_KAFKA_CONTROLLERID | |
| #if defined(HAS_RD_KAFKA_CONTROLLERID) && RD_KAFKA_VERSION >= 0x010000ff |
Then update the test SKIPIF section to check if the method exists.
There was a problem hiding this comment.
I've improved HAS_RD_KAFKA_CONTROLLERID the definition instead
Lines 74 to 82 in 015b7a2
|
failed tests are being flaky with |
|
Thank you! |
|
can we release a 6.0.4 version? |
|
bump @arnaud-lb 🙏 |
|
Released 6.0.4: https://github.com/arnaud-lb/php-rdkafka/releases/tag/6.0.4 |
No description provided.