You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by minifirocks <gi...@git.apache.org> on 2018/04/05 19:04:29 UTC
[GitHub] nifi-minifi-cpp pull request #295: MINFICPP-403: Flow Meta tagging
GitHub user minifirocks opened a pull request:
https://github.com/apache/nifi-minifi-cpp/pull/295
MINFICPP-403: Flow Meta tagging
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-XXXX where XXXX 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/minifirocks/nifi-minifi-cpp meta_info
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/nifi-minifi-cpp/pull/295.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 #295
----
commit 22f52fb9d225233ba63df04eb8c91ca9af94a4ba
Author: Bin Qiu <be...@...>
Date: 2018-04-03T15:57:23Z
MINFICPP-403: Flow Meta tagging
----
---
[GitHub] nifi-minifi-cpp issue #295: MINFICPP-403: Flow Meta tagging
Posted by phrocker <gi...@git.apache.org>.
Github user phrocker commented on the issue:
https://github.com/apache/nifi-minifi-cpp/pull/295
`#ifndef AGENT_BUILD_H
#define AGENT_BUILD_H
#include <vector>
namespace org {
namespace apache {
namespace nifi {
namespace minifi {
class AgentBuild {
public:
static constexpr const char* VERSION = "0.5.0";
static constexpr const char* BUILD_IDENTIFIER = "";
static constexpr const char* BUILD_REV = "b68d22ba55cfea069cbdbf7931e2b234d86b1f12";
static constexpr const char* BUILD_DATE = "1522951804";
static constexpr const char* COMPILER = "/Library/Developer/CommandLineTools/usr/bin/c++";
static constexpr const char* COMPILER_VERSION = "9.0.0.9000038";
static constexpr const char* COMPILER_FLAGS = " -std=c++11 -Wall";
static std::vector<std::string> getExtensions() {
static std::vector<std::string> extensions;
if (extensions.empty()){
extensions.push_back("minifi-expression-language-extensions");
extensions.push_back("minifi-http-curl");
extensions.push_back("minifi-civet-extensions");
extensions.push_back("minifi-rocksdb-repos");
extensions.push_back("minifi-archive-extensions");
extensions.push_back("minifi-gps");
extensions.push_back("minifi-mqtt-extensions");
extensions.push_back("minifi-pcap");
extensions.push_back("minifi-rdkafka-extensions");
extensions.push_back("minifi-script-extensions");
extensions.push_back("minifi-usb-camera-extensions");
}
return extensions;
}
};
} /* namespace minifi */
} /* namespace nifi */
} /* namespace apache */
} /* namespace org */
#endif /* AGENT_BUILD_H */
`
is an example.
---
[GitHub] nifi-minifi-cpp issue #295: MINFICPP-403: Flow Meta tagging
Posted by minifirocks <gi...@git.apache.org>.
Github user minifirocks commented on the issue:
https://github.com/apache/nifi-minifi-cpp/pull/295
@phrocker so if that's the case, we can remove the below meta info when the meta info container was constructed and just provide the framework, if user want to use that, they can create their own meta info and add this to the meta info container. **we provide a framework to let them add their own meta info into the flow file for mutable meta info.**
: config_(configure) {
+ // add version, serial number as default meta info
+ std::unique_ptr<core::MetaInfo> version = std::unique_ptr < core::MetaInfo > (new VersionMetaInfo());
+ addMetaInfo(std::move(version));
+ std::string serial_number;
+ config_->get("device.id", serial_number);
+ state::metrics::Device device;
+ if (serial_number.empty()) {
+ // we did not config serial number, use the mac address
+ serial_number = device.device_id_;
+ }
+ std::unique_ptr<core::MetaInfo> serial_number_meta_info = std::unique_ptr < core::MetaInfo >(new MetaInfo("device.id", serial_number));
+ addMetaInfo(std::move(serial_number_meta_info));
+ std::unique_ptr<core::MetaInfo> hostname_meta_info = std::unique_ptr < core::MetaInfo >(new MetaInfo("hostname", device.canonical_hostname_));
+ addMetaInfo(std::move(hostname_meta_info));
---
[GitHub] nifi-minifi-cpp issue #295: MINFICPP-403: Flow Meta tagging
Posted by minifirocks <gi...@git.apache.org>.
Github user minifirocks commented on the issue:
https://github.com/apache/nifi-minifi-cpp/pull/295
@phrocker the reason that i do not want to use processor is that meta info below to flow which is generated by the any processor. we provide a framework let user to write their own meta info and add to the container. for example, they can add vendor specified meta info. as for the shared ptr, i can change to unique ptr. i pick share ptr because it make it more flexible in case it need to be used somewhere besides the container.
---
[GitHub] nifi-minifi-cpp pull request #295: MINFICPP-403: Flow Meta tagging
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/nifi-minifi-cpp/pull/295
---
[GitHub] nifi-minifi-cpp issue #295: MINFICPP-403: Flow Meta tagging
Posted by minifirocks <gi...@git.apache.org>.
Github user minifirocks commented on the issue:
https://github.com/apache/nifi-minifi-cpp/pull/295
@phrocker Thanks for the review. the PR is not only for version, it provide a flexible framework to add other meta info also. also in the cmake file, we already specify major/minor/patch version, why we need another generateVersion.sh.
---
[GitHub] nifi-minifi-cpp pull request #295: MINFICPP-403: Flow Meta tagging
Posted by phrocker <gi...@git.apache.org>.
Github user phrocker commented on a diff in the pull request:
https://github.com/apache/nifi-minifi-cpp/pull/295#discussion_r179732500
--- Diff: libminifi/include/core/MetaInfo.h ---
@@ -0,0 +1,180 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+#ifndef __META_INFO_H__
+#define __META_INFO_H__
+
+#include <algorithm>
+#include <sstream>
+#include <string>
+#include <map>
+#include <mutex>
+#include <atomic>
+#include <functional>
+#include <set>
+#include <stdlib.h>
+#include <math.h>
+#include "utils/StringUtils.h"
+#include "core/FlowFile.h"
+#include "core/state/metrics/DeviceInformation.h"
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace core {
+
+// MetaInfo Class
+class MetaInfo {
+
+ public:
+ // Constructor
+ /*!
+ * Create a new meta info
+ */
+ explicit MetaInfo(const std::string &name, const std::string &value)
+ : name_(name),
+ value_(value) {
+ }
+
+ // Destructor
+ virtual ~MetaInfo() {
+ }
+
+ // Get Name for the meta info
+ std::string getName() {
+ return name_;
+ }
+ // Get value for the meta info, overwritten by specific child meta info
+ virtual std::string getValue(const std::shared_ptr<core::FlowFile> &flow) {
+ return value_;
+ }
+
+ protected:
+ // Name
+ std::string name_;
+ // Value
+ std::string value_;
+
+ private:
+};
+
+#ifdef MINIFI_VERSION
--- End diff --
This appears to be copied from stack overflow. Should probably change this .
---
[GitHub] nifi-minifi-cpp issue #295: MINFICPP-403: Flow Meta tagging
Posted by phrocker <gi...@git.apache.org>.
Github user phrocker commented on the issue:
https://github.com/apache/nifi-minifi-cpp/pull/295
@minifirocks per your comment twelve days ago you mentioned that you didn't want to use a processor because you're putting this on every flow file. That's pretty reasonable, but why do we need to use the attributes? This is something that's intended for immutable information, right? could we not have a static strings within the flow file for just the flow version that are serialized along with attributes?
If we do that we can let the other information be sent via C2 and correlated via provenance. I don't see the need to include meta information on every flow file for information that's immutable. Mutable information is better sent in the flow file because those immutable data points can be correlated when evaluating flow files from a given source.
---
[GitHub] nifi-minifi-cpp issue #295: MINFICPP-403: Flow Meta tagging
Posted by phrocker <gi...@git.apache.org>.
Github user phrocker commented on the issue:
https://github.com/apache/nifi-minifi-cpp/pull/295
@minifirocks that script is used with cmake to provide that information in a codified, statically defined way, to the code base. It's built at compile time and provides the agent with the information. Instead of using pragma definitions to pass the information it is statically defined. Additionally, the information this ticket is supposed to provide is flow information not agent information, correct?
---
[GitHub] nifi-minifi-cpp issue #295: MINFICPP-403: Flow Meta tagging
Posted by minifirocks <gi...@git.apache.org>.
Github user minifirocks commented on the issue:
https://github.com/apache/nifi-minifi-cpp/pull/295
@phrocker please review
change shared_ptr to unique ptr
use agent.version and flow.version for meta info
---
[GitHub] nifi-minifi-cpp issue #295: MINFICPP-403: Flow Meta tagging
Posted by minifirocks <gi...@git.apache.org>.
Github user minifirocks commented on the issue:
https://github.com/apache/nifi-minifi-cpp/pull/295
@phrocker please let me know whether you have more comments. Thanks.
---
[GitHub] nifi-minifi-cpp pull request #295: MINFICPP-403: Flow Meta tagging
Posted by minifirocks <gi...@git.apache.org>.
Github user minifirocks commented on a diff in the pull request:
https://github.com/apache/nifi-minifi-cpp/pull/295#discussion_r179779744
--- Diff: libminifi/include/core/MetaInfo.h ---
@@ -0,0 +1,180 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+#ifndef __META_INFO_H__
+#define __META_INFO_H__
+
+#include <algorithm>
+#include <sstream>
+#include <string>
+#include <map>
+#include <mutex>
+#include <atomic>
+#include <functional>
+#include <set>
+#include <stdlib.h>
+#include <math.h>
+#include "utils/StringUtils.h"
+#include "core/FlowFile.h"
+#include "core/state/metrics/DeviceInformation.h"
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace core {
+
+// MetaInfo Class
+class MetaInfo {
+
+ public:
+ // Constructor
+ /*!
+ * Create a new meta info
+ */
+ explicit MetaInfo(const std::string &name, const std::string &value)
+ : name_(name),
+ value_(value) {
+ }
+
+ // Destructor
+ virtual ~MetaInfo() {
+ }
+
+ // Get Name for the meta info
+ std::string getName() {
+ return name_;
+ }
+ // Get value for the meta info, overwritten by specific child meta info
+ virtual std::string getValue(const std::shared_ptr<core::FlowFile> &flow) {
+ return value_;
+ }
+
+ protected:
+ // Name
+ std::string name_;
+ // Value
+ std::string value_;
+
+ private:
+};
+
+#ifdef MINIFI_VERSION
--- End diff --
will do
---
[GitHub] nifi-minifi-cpp issue #295: MINFICPP-403: Flow Meta tagging
Posted by phrocker <gi...@git.apache.org>.
Github user phrocker commented on the issue:
https://github.com/apache/nifi-minifi-cpp/pull/295
I've been thinking more about this @minifirocks . I'm curious about the inception of this idea. I see that there is coupling amongst components that probably don't have a need to know for the metadata. In my opinion if you are putting the information in the attributes then it is potentially removable anyway, so why not use a Processor? If it's an immutable structure within the flow file then it makes sense to do it at construction -- and not in the attributes.
Further, any reason to use shared pointers for much of that meta information? Ownership seems like it would be well defined and thus should be coupled to the lifetime of that object. Move semantics (IMO) make more sense with that type of component lifetime, further compounded by the fact that they could be immutable.
---