[QC-1232] Improve log message when kafka broker url is missing #2437
Conversation
justonedev1
force-pushed
the
QC-1232
branch
from
1ed0ad5
to
afe5fd1
Compare
| { "enable.auto.commit", { "true" } }, | ||
| { "auto.offset.reset", { "latest" } } } }; | ||
| if (brokers.empty()) { | ||
| constexpr std::string_view message{ "You are trying to start KafkaPoller without any brokers" }; |
There was a problem hiding this comment.
why use a string_view here ?
| @@ -68,8 +68,25 @@ std::string createKafkaGroupId(std::string_view prefix, std::string_view detecto | |||
| return groupId; | |||
| } | |||
|
|
|||
| bool checkKafkaParams(const std::string& kafkaBrokers, const std::string& topic, const std::string_view triggerTypeLogId) | |||
| { | |||
There was a problem hiding this comment.
genuine question: why don't you use a string_view for the first two arguments ?
I understand that the parameters you will give later in SOR are strings, but from the point of view of the interface, wouldn't it be better to have something uniform ?
There was a problem hiding this comment.
you are right, all of these should be the same. Thanks for the remark
I generally prefer std::string_view over const std::string& because it is non owning and non allocating and read only class, so it 1) implies the usage, 2) allows you to use the function (generally speaking) allocating something/creating temporary string, 3) string_view can be used easily with wider range of classes: std::string, const char*, std::vector<char>, ... But in this case the choice does not really matter...
Co-authored-by: Piotr Konopka <piotr.jan.konopka@cern.ch>
better error handling when kafkaBrokers are missing