[rdkafka] remove topic conf, deprecated #1101
Conversation
|
So the failing test is due to the fact that you guys are using See my comment in #1103 , if you guys want to stick to |
|
@nick-zh is it ready ? |
|
If you want to go with my PR, maybe i could add an exception when somebody tries to use it with a lib version that is not supported, let me know |
|
yes, please |
|
will get right on it |
|
@makasim done, but i think you need to rebuild the docker image |
|
Seems there is more failing when trying to build the Dockerfile. I fixed it in #1104 to avoid mixing to much in this PR, hope that is ok ✌️ |
|
@makasim thx for merging the docker PR, give me a short shout when the image has been pushed to dockerhub, then i can merge master here and the tests should pass ✌️ |
| @@ -68,6 +68,7 @@ install: | |||
| - echo "memory_limit=2048M" >> ~/.phpenv/versions/$(phpenv version-name)/etc/conf.d/travis.ini | |||
| - php ./bin/fix-symfony-version.php "$SYMFONY_VERSION" | |||
| - composer install | |||
| - sed -i 's/525568/16777471/' vendor/kwn/php-rdkafka-stubs/stubs/constants.php | |||
There was a problem hiding this comment.
this is needed for the tests, since the stub has a really old lib version, otherwise the new check will trigger
There was a problem hiding this comment.
A comment near the line would be great.
|
@makasim PR should be good to go now. The failing tests are G-PubSub related, from what i can see, this happened before my changes as well |
This will fix #970, the use of topic conf is deprecated, all configs should be set over the global config.
To avoid a break / major, i adapted this internally where config on both levels (global, topic) will be treated / set equally.