You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by phrocker <gi...@git.apache.org> on 2018/08/04 18:31:04 UTC
[GitHub] nifi-minifi-cpp pull request #388: MINIFICPP-590: Fix zero length files at s...
GitHub user phrocker opened a pull request:
https://github.com/apache/nifi-minifi-cpp/pull/388
MINIFICPP-590: Fix zero length files at startup
MINIFICPP-589: Institute regex filtering for interfaces
MINIFICPP-589: Limit Pcap to 4.9.4+
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/phrocker/nifi-minifi-cpp MINIFICPP-590
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/nifi-minifi-cpp/pull/388.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 #388
----
commit 6a345c9e94f2adfdfea4c4bc97dd9c3532d5f789
Author: Marc Parisi <ph...@...>
Date: 2018-08-04T18:26:54Z
MINIFICPP-590: Fix zero length files at startup
MINIFICPP-589: Institute regex filtering for interfaces
MINIFICPP-589: Limit Pcap to 4.9.4+
----
---
[GitHub] nifi-minifi-cpp pull request #388: MINIFICPP-590: Fix zero length files at s...
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/388#discussion_r208026845
--- Diff: extensions/pcap/CapturePacket.cpp ---
@@ -54,13 +54,15 @@ namespace processors {
std::shared_ptr<utils::IdGenerator> CapturePacket::id_generator_ = utils::IdGenerator::getIdGenerator();
core::Property CapturePacket::BaseDir("Base Directory", "Scratch directory for PCAP files", "/tmp/");
core::Property CapturePacket::BatchSize("Batch Size", "The number of packets to combine within a given PCAP", "50");
+core::Property CapturePacket::NetworkControllers("Network Controllers", "List of network controllers to attach to -- each may be a regex", ".*");
--- End diff --
This is meant to convey a YAML list ( https://docs.ansible.com/ansible/latest/reference_appendices/YAMLSyntax.html ), however as this isn't intended to be YAML specific I will clarify this. I thought about making it dynamic but did not see great value. Do you think this should be dynamic?
---
[GitHub] nifi-minifi-cpp issue #388: MINIFICPP-590: Fix zero length files at startup
Posted by phrocker <gi...@git.apache.org>.
Github user phrocker commented on the issue:
https://github.com/apache/nifi-minifi-cpp/pull/388
@apiri also created https://issues.apache.org/jira/browse/MINIFICPP-594 since I think the documentation was created after the CapturePacket processor. Happy to combine the PRs if you prefer, but otherwise will submit that as a separate PR tomorow .
---
[GitHub] nifi-minifi-cpp issue #388: MINIFICPP-590: Fix zero length files at startup
Posted by phrocker <gi...@git.apache.org>.
Github user phrocker commented on the issue:
https://github.com/apache/nifi-minifi-cpp/pull/388
Did not experience issue noted in MINIFICPP-590. The only issue were zero length files at predictable times due to memory collisions. Batching appears to work as designed.
---
[GitHub] nifi-minifi-cpp pull request #388: MINIFICPP-590: Fix zero length files at s...
Posted by apiri <gi...@git.apache.org>.
Github user apiri commented on a diff in the pull request:
https://github.com/apache/nifi-minifi-cpp/pull/388#discussion_r207919579
--- Diff: extensions/pcap/CapturePacket.cpp ---
@@ -54,13 +54,15 @@ namespace processors {
std::shared_ptr<utils::IdGenerator> CapturePacket::id_generator_ = utils::IdGenerator::getIdGenerator();
core::Property CapturePacket::BaseDir("Base Directory", "Scratch directory for PCAP files", "/tmp/");
core::Property CapturePacket::BatchSize("Batch Size", "The number of packets to combine within a given PCAP", "50");
+core::Property CapturePacket::NetworkControllers("Network Controllers", "List of network controllers to attach to -- each may be a regex", ".*");
--- End diff --
the way this is worded is a little awkward. I understand this could resolve to multiple interfaces with the regex but this would make me think I could do a csv of regexes although it seems that this would be done by having multiple instances of that property. is the intent to have dynamic properties for this?
---
[GitHub] nifi-minifi-cpp pull request #388: MINIFICPP-590: Fix zero length files at s...
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/388#discussion_r208027287
--- Diff: extensions/pcap/CapturePacket.cpp ---
@@ -54,13 +54,15 @@ namespace processors {
std::shared_ptr<utils::IdGenerator> CapturePacket::id_generator_ = utils::IdGenerator::getIdGenerator();
core::Property CapturePacket::BaseDir("Base Directory", "Scratch directory for PCAP files", "/tmp/");
core::Property CapturePacket::BatchSize("Batch Size", "The number of packets to combine within a given PCAP", "50");
+core::Property CapturePacket::NetworkControllers("Network Controllers", "List of network controllers to attach to -- each may be a regex", ".*");
core::Property CapturePacket::CaptureBluetooth("Capture Bluetooth", "True indicates that we support bluetooth interfaces", "false");
const char *CapturePacket::ProcessorName = "CapturePacket";
std::string CapturePacket::generate_new_pcap(const std::string &base_path) {
std::string path = base_path;
// can use relaxed for a counter
+ //int cnt = num_.fetch_add(1, std::memory_order_relaxed);
--- End diff --
err thanks can't believe I missed that.
---
[GitHub] nifi-minifi-cpp issue #388: MINIFICPP-590: Fix zero length files at startup
Posted by apiri <gi...@git.apache.org>.
Github user apiri commented on the issue:
https://github.com/apache/nifi-minifi-cpp/pull/388
As long as we have a ticket for it that works for me. Will review the updates.
---
[GitHub] nifi-minifi-cpp pull request #388: MINIFICPP-590: Fix zero length files at s...
Posted by apiri <gi...@git.apache.org>.
Github user apiri commented on a diff in the pull request:
https://github.com/apache/nifi-minifi-cpp/pull/388#discussion_r208041393
--- Diff: extensions/pcap/CapturePacket.cpp ---
@@ -54,13 +54,15 @@ namespace processors {
std::shared_ptr<utils::IdGenerator> CapturePacket::id_generator_ = utils::IdGenerator::getIdGenerator();
core::Property CapturePacket::BaseDir("Base Directory", "Scratch directory for PCAP files", "/tmp/");
core::Property CapturePacket::BatchSize("Batch Size", "The number of packets to combine within a given PCAP", "50");
+core::Property CapturePacket::NetworkControllers("Network Controllers", "List of network controllers to attach to -- each may be a regex", ".*");
--- End diff --
I would agree that dynamic doesn't really seem like the right fit and arguably a misuse of what they are supposed to represent. The clarification is definitely helpful though concerning the expectation of the YAML list. Admittedly this comment stemmed from my NiFI leaning perspective where all property values are inherently just strings
---
[GitHub] nifi-minifi-cpp pull request #388: MINIFICPP-590: Fix zero length files at s...
Posted by apiri <gi...@git.apache.org>.
Github user apiri commented on a diff in the pull request:
https://github.com/apache/nifi-minifi-cpp/pull/388#discussion_r207762481
--- Diff: extensions/pcap/CapturePacket.cpp ---
@@ -54,13 +54,15 @@ namespace processors {
std::shared_ptr<utils::IdGenerator> CapturePacket::id_generator_ = utils::IdGenerator::getIdGenerator();
core::Property CapturePacket::BaseDir("Base Directory", "Scratch directory for PCAP files", "/tmp/");
core::Property CapturePacket::BatchSize("Batch Size", "The number of packets to combine within a given PCAP", "50");
+core::Property CapturePacket::NetworkControllers("Network Controllers", "List of network controllers to attach to -- each may be a regex", ".*");
core::Property CapturePacket::CaptureBluetooth("Capture Bluetooth", "True indicates that we support bluetooth interfaces", "false");
const char *CapturePacket::ProcessorName = "CapturePacket";
std::string CapturePacket::generate_new_pcap(const std::string &base_path) {
std::string path = base_path;
// can use relaxed for a counter
+ //int cnt = num_.fetch_add(1, std::memory_order_relaxed);
--- End diff --
we can nix this
---
[GitHub] nifi-minifi-cpp pull request #388: MINIFICPP-590: Fix zero length files at s...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/nifi-minifi-cpp/pull/388
---