search

etf1 / kafka-message-scheduler

scheduler for kafka messages
MIT License
76 stars 14 forks source link

Support nil target keys #26

Closed james-johnston-thumbtack closed 2 years ago

james-johnston-thumbtack commented 2 years ago

Messages that are scheduled with a nil target key are currently being inadvertently converted to messages with a zero length string target key.

Fix the core scheduling logic so that nil values are retained.

Fixes #24

james-johnston-thumbtack commented 2 years ago

@fkarakas the build check seems to have failed due to reasons unrelated to this commit, if I'm interpreting them right:

tests_1      | Waiting for kafka cluster to be ready ...
tests_1      | Waiting for kafka cluster to be ready ...
tests_1      | Waiting for kafka cluster to be ready ...
tests_1      | Waiting for kafka cluster to be ready ...
tests_1      | Waiting for kafka cluster to be ready ...
tests_1      | Waiting for kafka cluster to be ready ...
tests_1      | Waiting for kafka cluster to be ready ...
tests_1      | Waiting for kafka cluster to be ready ...
tests_1      | Waiting for kafka cluster to be ready ...
tests_1      | Waiting for kafka cluster to be ready ...
tests_tests_1 exited with code [127](https://github.com/etf1/kafka-message-scheduler/runs/6227560488?check_suite_focus=true#step:3:127)

Any thoughts on what the next steps with this pull request should be? Thanks.

fkarakas commented 2 years ago

Hello @james-johnston-thumbtack thanks for the PR, i am currently on vacation. I will check the code and failing test next week. Otherwise if you can add tests for the yaml config and nil target key i will appreciate it. If you dont know how to, dont worry we can do that next week. Thank you your contribution is very good 👌