Skip to content

MINFICPP-403: Flow Meta tagging#295

Closed
minifirocks wants to merge 1 commit intoapache:masterfrom
minifirocks:meta_info
Closed

MINFICPP-403: Flow Meta tagging#295
minifirocks wants to merge 1 commit intoapache:masterfrom
minifirocks:meta_info

Conversation

@minifirocks
Copy link

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?
  • 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.

Copy link
Contributor

@phrocker phrocker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://github.com/phrocker/nifi-minifi-cpp/blob/C2FINISH/generateVersion.sh

That is an approach that can work to propagate this information statically without making changes anywhere except ProcessContext.

@minifirocks
Copy link
Author

@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.

@phrocker
Copy link
Contributor

phrocker commented Apr 6, 2018

@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?

private:
};

#ifdef MINIFI_VERSION
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This appears to be copied from stack overflow. Should probably change this .

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will do

@phrocker
Copy link
Contributor

phrocker commented Apr 6, 2018

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.

@minifirocks
Copy link
Author

@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.

@minifirocks
Copy link
Author

@phrocker please review
change shared_ptr to unique ptr
use agent.version and flow.version for meta info

@minifirocks
Copy link
Author

@phrocker please let me know whether you have more comments. Thanks.

@phrocker
Copy link
Contributor

@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.

@minifirocks
Copy link
Author

@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_ptrcore::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_ptrcore::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_ptrcore::MetaInfo hostname_meta_info = std::unique_ptr < core::MetaInfo >(new MetaInfo("hostname", device.canonical_hostname_));
  • addMetaInfo(std::move(hostname_meta_info));

@asfgit asfgit closed this in 02f1823 May 18, 2018
nghiaxlee pushed a commit to nghiaxlee/nifi-minifi-cpp that referenced this pull request Jul 8, 2019
…o the flow

identifier. With this approach the flow identifier will be updated with C2
and automatically apply to any processors applied as a result of that update

This closes apache#331.

closes apache#313
closes apache#295

Signed-off-by: Aldrin Piri <aldrin@apache.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants