Skip to content

MINIFI-159: Create AppendHostInfo processor#27

Closed
randerzander wants to merge 3 commits intoapache:masterfrom
randerzander:master
Closed

MINIFI-159: Create AppendHostInfo processor#27
randerzander wants to merge 3 commits intoapache:masterfrom
randerzander:master

Conversation

@randerzander
Copy link
Contributor

No description provided.

@randerzander
Copy link
Contributor Author

I believe the CI failure is due to broken appveyor build configuration

@apiri
Copy link
Member

apiri commented Dec 15, 2016

reviewing

Copy link
Member

@apiri apiri left a comment

Choose a reason for hiding this comment

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

Hey @randerzander,

Overall looks good and works as anticipated. Had a few comments. Let me know your thoughts.

return;

//Get Hostname
char hostname[1024];
Copy link
Member

Choose a reason for hiding this comment

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

Is there a reason 1024 is used? By spec, it is no more than 255 but may be lower depending on OS implementation. limits.h has a HOST_NAME_MAX that contains this to get more precision on this instantiation

flow->addAttribute(hostAttribute.c_str(), h->h_name);

//Get IP address for the specified interface
std::string iface;
Copy link
Member

Choose a reason for hiding this comment

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

What is the appropriate path if the interface specified does not exist? Before adjusting, this currently returns 1.0.0.0. Would it make sense, should the interface not be found, to omit the inclusion of the attribute and value?

@apiri
Copy link
Member

apiri commented Dec 16, 2016

Hey @randerzander. Changes look good. Will get this merged in. Thanks for the contrib, definitely seems like it would be helpful.

@asfgit asfgit closed this in e9cfbfe Dec 16, 2016
phrocker pushed a commit to phrocker/nifi-minifi-cpp that referenced this pull request Feb 10, 2017
This closes apache#27.

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