[GitHub] nifi-minifi-cpp pull request #360: MINIFICPP-427 - add PublishKafka headers ...
Github user asfgit closed the pull request at: https://github.com/apache/nifi-minifi-cpp/pull/360 ---
[GitHub] nifi-minifi-cpp pull request #360: MINIFICPP-427 - add PublishKafka headers ...
Github user phrocker commented on a diff in the pull request: https://github.com/apache/nifi-minifi-cpp/pull/360#discussion_r205800065 --- Diff: extensions/librdkafka/PublishKafka.h --- @@ -28,6 +28,7 @@ #include "core/Property.h" #include "core/logging/LoggerConfiguration.h" #include "rdkafka.h" +#include --- End diff -- @dtrodrigues Did you need help with limiting to 4.9 insofar that we can limit it in the cmake and documentation? ---
[GitHub] nifi-minifi-cpp pull request #360: MINIFICPP-427 - add PublishKafka headers ...
Github user phrocker commented on a diff in the pull request: https://github.com/apache/nifi-minifi-cpp/pull/360#discussion_r198911858 --- Diff: extensions/librdkafka/PublishKafka.h --- @@ -28,6 +28,7 @@ #include "core/Property.h" #include "core/logging/LoggerConfiguration.h" #include "rdkafka.h" +#include --- End diff -- I'm fine with limiting it to 4.9 forward. ---
[GitHub] nifi-minifi-cpp pull request #360: MINIFICPP-427 - add PublishKafka headers ...
Github user dtrodrigues commented on a diff in the pull request: https://github.com/apache/nifi-minifi-cpp/pull/360#discussion_r198361089 --- Diff: extensions/librdkafka/PublishKafka.h --- @@ -28,6 +28,7 @@ #include "core/Property.h" #include "core/logging/LoggerConfiguration.h" #include "rdkafka.h" +#include --- End diff -- I believe GCC prior to 4.9 didn't support C++11 regex. I'm leaning towards not supporting older compilers for this processor but can look into using regex.h instead if desired. ---
[GitHub] nifi-minifi-cpp pull request #360: MINIFICPP-427 - add PublishKafka headers ...
Github user dtrodrigues commented on a diff in the pull request:
https://github.com/apache/nifi-minifi-cpp/pull/360#discussion_r198360594
--- Diff: extensions/librdkafka/PublishKafka.h ---
@@ -115,14 +121,35 @@ class PublishKafka: public core::Processor {
buffer.reserve(max_seg_size_);
read_size_ = 0;
status_ = 0;
+ rd_kafka_resp_err_t err;
+
+ for (auto kv : flowFile_->getAttributes()) {
+if (regex_match(kv.first, attributeNameRegex_)) {
+ if (!hdrs) {
+hdrs = rd_kafka_headers_new(8);
--- End diff --
8 was mostly arbitrary, I used the value from
https://github.com/edenhill/librdkafka/blob/master/examples/rdkafka_example.c#L416
since some value needed to be used. 0 is also an option but it looks like if 0
is chosen, any additions will grow the list to 16 elements. If a number besides
0 is chosen, when list runs out of room, the list is grown by 2*the list size.
---
[GitHub] nifi-minifi-cpp pull request #360: MINIFICPP-427 - add PublishKafka headers ...
Github user dtrodrigues commented on a diff in the pull request: https://github.com/apache/nifi-minifi-cpp/pull/360#discussion_r198359374 --- Diff: extensions/librdkafka/PublishKafka.cpp --- @@ -262,7 +269,7 @@ void PublishKafka::onTrigger(const std::shared_ptr &contex if (flowFile->getAttribute(KAFKA_KEY_ATTRIBUTE, value)) kafkaKey = value; - PublishKafka::ReadCallback callback(flowFile->getSize(), max_seg_size_, kafkaKey, rkt_); + PublishKafka::ReadCallback callback(flowFile->getSize(), max_seg_size_, kafkaKey, rkt_, rk_, flowFile, attributeNameRegex); --- End diff -- Now that we're sending the flowFile so we have access to the attributes, we don't need to send the size by itself anymore. I'll update this. ---
[GitHub] nifi-minifi-cpp pull request #360: MINIFICPP-427 - add PublishKafka headers ...
Github user phrocker commented on a diff in the pull request: https://github.com/apache/nifi-minifi-cpp/pull/360#discussion_r197826693 --- Diff: extensions/librdkafka/PublishKafka.cpp --- @@ -262,7 +269,7 @@ void PublishKafka::onTrigger(const std::shared_ptr &contex if (flowFile->getAttribute(KAFKA_KEY_ATTRIBUTE, value)) kafkaKey = value; - PublishKafka::ReadCallback callback(flowFile->getSize(), max_seg_size_, kafkaKey, rkt_); + PublishKafka::ReadCallback callback(flowFile->getSize(), max_seg_size_, kafkaKey, rkt_, rk_, flowFile, attributeNameRegex); --- End diff -- Do we need to send the size and the flow file? ---
[GitHub] nifi-minifi-cpp pull request #360: MINIFICPP-427 - add PublishKafka headers ...
Github user phrocker commented on a diff in the pull request:
https://github.com/apache/nifi-minifi-cpp/pull/360#discussion_r197824984
--- Diff: extensions/librdkafka/PublishKafka.h ---
@@ -115,14 +121,35 @@ class PublishKafka: public core::Processor {
buffer.reserve(max_seg_size_);
read_size_ = 0;
status_ = 0;
+ rd_kafka_resp_err_t err;
+
+ for (auto kv : flowFile_->getAttributes()) {
+if (regex_match(kv.first, attributeNameRegex_)) {
+ if (!hdrs) {
+hdrs = rd_kafka_headers_new(8);
--- End diff --
Was eight randomly chosen? If not please add a comment for any maintainers
to understand why you chose this number -- even if any number can be added or
removed per their API.
---
[GitHub] nifi-minifi-cpp pull request #360: MINIFICPP-427 - add PublishKafka headers ...
Github user phrocker commented on a diff in the pull request: https://github.com/apache/nifi-minifi-cpp/pull/360#discussion_r197824393 --- Diff: extensions/librdkafka/PublishKafka.h --- @@ -28,6 +28,7 @@ #include "core/Property.h" #include "core/logging/LoggerConfiguration.h" #include "rdkafka.h" +#include --- End diff -- This does not work with all variants of GCC that we support. Therefore you will either need to switch to regex.h ( C variant ) or put a note in the ReadMe that not all versions of gcc ( I don't recall, but remember the issues with GetFile ) may support this processor. Will then require some checks in bootstrap to avoid users building this suite of processors for those versions. When I have some time and am back home I can attempt a build across the variants. I am leaning toward 4.8.4 having issues, so I will attempt that first. ---
[GitHub] nifi-minifi-cpp pull request #360: MINIFICPP-427 - add PublishKafka headers ...
GitHub user dtrodrigues opened a pull request: https://github.com/apache/nifi-minifi-cpp/pull/360 MINIFICPP-427 - add PublishKafka headers support Thank you for submitting a contribution to Apache NiFi - MiNiFi C++. In order to streamline the review of the contribution we ask you to ensure the following steps have been taken: ### For all changes: - [ ] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message? - [ ] Does your PR title start with MINIFI- where is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character. - [ ] Has your PR been rebased against the latest commit within the target branch (typically master)? - [ ] Is your initial contribution a single, squashed commit? ### For code changes: - [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)? - [ ] If applicable, have you updated the LICENSE file? - [ ] If applicable, have you updated the NOTICE file? ### For documentation related changes: - [ ] Have you ensured that format looks appropriate for the output in which it is rendered? ### Note: Please ensure that once the PR is submitted, you check travis-ci for build issues and submit an update to your PR as soon as possible. You can merge this pull request into a Git repository by running: $ git pull https://github.com/dtrodrigues/nifi-minifi-cpp MINIFICPP-427 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/nifi-minifi-cpp/pull/360.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #360 commit 7c4a8f2e5a4d1bb4335c0d46ca26e07e8b48f7a3 Author: Dustin Rodrigues Date: 2018-06-16T16:46:09Z MINIFICPP-427 - add PublishKafka headers support ---
