Github user ottobackwards commented on the issue:
https://github.com/apache/metron/pull/530
Note to all reviewers:
https://github.com/apache/metron/pull/720
and the new feature branch have been created.
I believe I have addressed everything brought up here at this
Github user ottobackwards commented on the issue:
https://github.com/apache/metron/pull/530
closing this in preparation for feature branch
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this
Github user merrimanr commented on the issue:
https://github.com/apache/metron/pull/530
There are some consequences of not having the bundle classes available in
rest. The list of available classes will only contain metron-parsers homed
parser types so those are the only type of pars
Github user ottobackwards commented on the issue:
https://github.com/apache/metron/pull/530
And also, the PR for fixing grok needs to go in to do the next step there
as well.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as wel
Github user ottobackwards commented on the issue:
https://github.com/apache/metron/pull/530
> NOTE On current configure UI limitations
Because I was already tasked with making this PR smaller ( don't laugh ), I
left the Configuration UI and REST not integrated with Bundles. T
Github user ottobackwards commented on the issue:
https://github.com/apache/metron/pull/530
OK @merrimanr , @mmiklavc
I have resolved the issues for working with grok from the rest/ui.
[PR INTO 777 for fixing
GROK](https://github.com/ottobackwards/metron/pull/6)
Github user ottobackwards commented on the issue:
https://github.com/apache/metron/pull/530
Here is the current status
- bundles-lib and maven plugin review is almost done.
- testing has shown both a bug in grok rules from hdfs in my implementation
and in the system as wel
Github user ottobackwards commented on the issue:
https://github.com/apache/metron/pull/530
Thank you @mattf-horton.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and w
Github user mattf-horton commented on the issue:
https://github.com/apache/metron/pull/530
And @ottobackwards , I want to say thank you for all your patience plowing
thru this stuff, with unavoidably slow rates of review. It's been a monumental
contribution, and I look forward to see
Github user mattf-horton commented on the issue:
https://github.com/apache/metron/pull/530
Looks good, you've responded to all my comments satisfactorily, and Travis
is still happy.
You have my +1, contingent on @mmiklavc 's approval, since he's doing the
practical testing.
---
Github user mattf-horton commented on the issue:
https://github.com/apache/metron/pull/530
@ottobackwards , looking very good. Couple more comments above
(unfortunately the system decided they are on "outdated" code, so you'll have
to press the "Show outdated" buttons), then you have
Github user ottobackwards commented on the issue:
https://github.com/apache/metron/pull/530
I don't think so. Are you running the latest? I'll run up full dev and
check.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well.
Github user mmiklavc commented on the issue:
https://github.com/apache/metron/pull/530
I'm seeing this error spinning up topologies in full-dev.
```
1:50.278 o.a.s.util [ERROR] Async loop died!
java.lang.RuntimeException: Grok parser Error: Grok parser unable to
initialize
Github user ottobackwards commented on the issue:
https://github.com/apache/metron/pull/530
@mattf-horton all feedback addressed, as noted above
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not hav
Github user mattf-horton commented on the issue:
https://github.com/apache/metron/pull/530
@ottobackwards , many good improvements here. A few comments on the
singleton idiom, and a couple other details.
---
If your project is set up for it, you can reply to this email and have your
Github user ottobackwards commented on the issue:
https://github.com/apache/metron/pull/530
@mattf-horton I really like how this is evolving. One thing I have been
thinking of since adding the BundleSystem interface ( which should be the main
external interface ) is that I would like
Github user ottobackwards commented on the issue:
https://github.com/apache/metron/pull/530
I have refactored the BundleThreadContextClassLoader, and removed the
mechanics around initializing it.
---
If your project is set up for it, you can reply to this email and have your
reply ap
Github user ottobackwards commented on the issue:
https://github.com/apache/metron/pull/530
*[bundles-lib/src/main/java/org/apache/metron/bundles/BundleThreadContextClassLoader.java,
line 43 at
r2](https://reviewable.io:443/reviews/apache/metro
Github user ottobackwards commented on the issue:
https://github.com/apache/metron/pull/530
*[bundles-lib/src/main/java/org/apache/metron/bundles/ExtensionManager.java,
line 255 at
r2](https://reviewable.io:443/reviews/apache/metron/530#-Kr7wzD
Github user ottobackwards commented on the issue:
https://github.com/apache/metron/pull/530
*[bundles-lib/src/main/java/org/apache/metron/bundles/VFSBundleClassLoader.java,
line 174 at
r2](https://reviewable.io:443/reviews/apache/metron/530#-Kr
Github user ottobackwards commented on the issue:
https://github.com/apache/metron/pull/530
*[bundles-lib/src/main/java/org/apache/metron/bundles/BundleThreadContextClassLoader.java,
line 117 at
r2](https://reviewable.io:443/reviews/apache/metr
Github user ottobackwards commented on the issue:
https://github.com/apache/metron/pull/530
@mattf-horton please take a look at
[22ba18b](https://github.com/apache/metron/pull/530/commits/22ba18b569b05bf040d1fca1f331602210e2e5ec)
[97ef4f0](https://github.com/apache/metron/pul
Github user ottobackwards commented on the issue:
https://github.com/apache/metron/pull/530
Thanks Matt. I will go through and reply later tonight.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does no
Github user ottobackwards commented on the issue:
https://github.com/apache/metron/pull/530
I will also post the review issues that I do not think have been touched on
yet
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well.
Github user mattf-horton commented on the issue:
https://github.com/apache/metron/pull/530
Okay, @ottobackwards , after all this I really wanted to just say "it's
good" :-) but I did have a few more concerns, especially with
ExtensionManager.java.
I want to emphasize that this i
Github user ottobackwards commented on the issue:
https://github.com/apache/metron/pull/530
in fact, you will see more in the management ui, since the extension
parsers all have default configurations and will be present :)
---
If your project is set up for it, you can reply to this
Github user ottobackwards commented on the issue:
https://github.com/apache/metron/pull/530
The management ui works with zookeeper. You will be able to work with it.
Adding support for installing extensions through the UI is a follow on, but I
am not sure that I can do it
---
If y
Github user mmiklavc commented on the issue:
https://github.com/apache/metron/pull/530
Ok, that's fine. Just thinking out loud. I'm considering your comments
about core changes vs the work in METRON-942. So with the combination of this
PR and 942, where does this leave the management
Github user ottobackwards commented on the issue:
https://github.com/apache/metron/pull/530
We discussed this higher up in the pr and decided not to
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not
Github user mmiklavc commented on the issue:
https://github.com/apache/metron/pull/530
Does it make sense to create a feature branch for these PR's?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not
Github user ottobackwards commented on the issue:
https://github.com/apache/metron/pull/530
@mmiklavc keep in mind that the proper scope for the document is building
new parsers that are 'part' of metron for METRON-777.
METRON-942 has the capability to actually install and dep
Github user mmiklavc commented on the issue:
https://github.com/apache/metron/pull/530
Hey @ottobackwards. I see no benefit to getting this committed prior to or
separate from shoring up the docs. I'm going to work on something to get the
ball rolling as I work through some use case
Github user ottobackwards commented on the issue:
https://github.com/apache/metron/pull/530
Test
---
*Comments from
[Reviewable](https://reviewable.io:443/reviews/apache/metron/530#-:-Kr8-4J5YPoUugdlItUi:bb74njr)*
---
If your proj
Github user ottobackwards commented on the issue:
https://github.com/apache/metron/pull/530
Yes, and there were not a lot in NAR to start with
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have
Github user mattf-horton commented on the issue:
https://github.com/apache/metron/pull/530
Sure, no worries. And I didn't intend to imply that testing was
inadequate, just suggesting another for completeness. Can't have too many
tests :-)
---
If your project is set up for it, you
Github user ottobackwards commented on the issue:
https://github.com/apache/metron/pull/530
Yes, I am sorry, I just wanted to point out another test, outside the area
that you are currently looking. I did not mean to imply that it negated the
need for 1099.
Although, if we h
Github user mattf-horton commented on the issue:
https://github.com/apache/metron/pull/530
@ottobackwards , re `metron-parser-bundle-tests`, very good to have that
test. But it only loads one test bundle, right? so still would be good to
implement METRON-1099. Emphasizing that isn'
Github user ottobackwards commented on the issue:
https://github.com/apache/metron/pull/530
@mmiklavc The documentation also changes with METRON-942, as that includes
the REST installation steps. If we can get these two PR's through, then follow
on with improved docs, it may make bet
Github user ottobackwards commented on the issue:
https://github.com/apache/metron/pull/530
@mmiklavc can the new document be a follow on? The jira with your writeup
would be a good one.
---
If your project is set up for it, you can reply to this email and have your
reply appear on
Github user ottobackwards commented on the issue:
https://github.com/apache/metron/pull/530
@mmiklavc "useful and overwhelming at the same time". If I had a nickelâ¢.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If y
Github user ottobackwards commented on the issue:
https://github.com/apache/metron/pull/530
@mattf-horton I don't know if you have seen it, but there is an integration
test that tests parser but ensures that the bundle is loaded and not in the
default classloader...
metron-p
Github user mmiklavc commented on the issue:
https://github.com/apache/metron/pull/530
Got it. Thanks @ottobackwards.
I want to repeat my appreciation for this contribution. I know it's taking
us time to get through review, and there's a lot of work in merging ongoing
change
Github user mattf-horton commented on the issue:
https://github.com/apache/metron/pull/530
@ottobackwards , the \@VisibleForTesting annotation comes from:
> import com.google.common.annotations.VisibleForTesting;
which I believe comes from
```xml
18.0
Github user mattf-horton commented on the issue:
https://github.com/apache/metron/pull/530
I've opened METRON-1099 for integration tests regarding the two items I'm
not sure from code inspection will work right. But I'm not making this review
dependent on them because they are a sign
Github user ottobackwards commented on the issue:
https://github.com/apache/metron/pull/530
@mattf-horton , @mmiklavc
The last commit has changes to address your review feedback.
Matt, I was unable to get the @VisibleForTest to work out of package the
way I needed it to, so I
Github user ottobackwards commented on the issue:
https://github.com/apache/metron/pull/530
@mattf-horton I will have a commit addressing all your review points soon.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If you
Github user mmiklavc commented on the issue:
https://github.com/apache/metron/pull/530
This looks much better now. **Note, I also changed the archetype packaging
version to 3.0.1 as well since this is the latest.**
```
{11:39}/tmp â mvn archetype:generate -DarchetypeCata
Github user mmiklavc commented on the issue:
https://github.com/apache/metron/pull/530
I think I've figured out what's going on with the plugin versions. I was
under the impression that Maven updated its local metadata on at least a daily
basis. It would appear that this is not true.
Github user mattf-horton commented on the issue:
https://github.com/apache/metron/pull/530
@ottobackwards , this is great work! Sorry the review's so long, and it
still isn't quite done. But some good news: I've fully reviewed
- bundles-maven-plugin
- metron-maven-archetype
Github user mmiklavc commented on the issue:
https://github.com/apache/metron/pull/530
Running the generate command in CentOS results in defaulting to plugin
version 3.0.1 when run from a directory without a pom. In Mac, I'm getting 2.4
for some reason. I haven't found any settings ye
Github user mmiklavc commented on the issue:
https://github.com/apache/metron/pull/530
I just noticed that you had addressed my findings around the plugin version
as well. So I think going back to 3.0.1 is ok for now. We do need to figure out
what is going on with Mac OS before gettin
Github user ottobackwards commented on the issue:
https://github.com/apache/metron/pull/530
I have changed it back to 3.0.1.
I have also resolved the issue that required the metron version and
archetype version to match in order to build.
---
If your project is set up for it, yo
Github user ottobackwards commented on the issue:
https://github.com/apache/metron/pull/530
@mmiklavc does that mean we can go back to 3.0.1? Or should?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project doe
Github user mmiklavc commented on the issue:
https://github.com/apache/metron/pull/530
Ok, I decided to try yet another combo here. I set the plugin version back
to 3.0.1 and re-installed the archetype. Now I get a new file
`~/.m2/repository/archetype-catalog.xml` with the archetype i
Github user mmiklavc commented on the issue:
https://github.com/apache/metron/pull/530
So another wrinkle here - when running the archetype plugin from a
non-project (separate, clean) directory, it's going to default to 3.0.1. I'm
trying this up in full dev right now and running into
Github user ottobackwards commented on the issue:
https://github.com/apache/metron/pull/530
ok - so mvn archetype:generate uses the newer version of the plugin if run
in an empty directory.
If run in a directory where there is a pom with plugin setting for 2.4 and
it will honor th
Github user ottobackwards commented on the issue:
https://github.com/apache/metron/pull/530
hey @mmiklavc . Looking back at my logs above, no matter that the
maven-archetype-plugin was 2.4 in the pom at the time, it shows 3.0.1 in the
output. Can you check what version it shows in y
Github user ottobackwards commented on the issue:
https://github.com/apache/metron/pull/530
I also may have a fix for the missing catalog file issue ( or a way to run
the command differently )
---
If your project is set up for it, you can reply to this email and have your
reply appea
Github user ottobackwards commented on the issue:
https://github.com/apache/metron/pull/530
Ok, I tried with a new M2 directory and the can't find local archetype
issue returned.
maven-archetype-plugin
3.0.1
Github user ottobackwards commented on the issue:
https://github.com/apache/metron/pull/530
I'm going to try with a new .m2 dir
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
en
Github user ottobackwards commented on the issue:
https://github.com/apache/metron/pull/530
I hate to ask but are you sure you have the latest code?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not
Github user mmiklavc commented on the issue:
https://github.com/apache/metron/pull/530
Hrm, I completely blew away both archetype catalogs and re-installed our
archetype again. It shows up with the correct version. However, the parser
folders are still not being created correctly. As
Github user ottobackwards commented on the issue:
https://github.com/apache/metron/pull/530
```bash
ââ[ottofowler@Winterfell] - [~/tmp/HelloParser] - [Thu Aug 03, 19:12]
ââ[$]> cat ~/.m2/repository/archetype-catalog.xml
http://maven.apache.org/plugins/maven-arch
Github user ottobackwards commented on the issue:
https://github.com/apache/metron/pull/530
I left the Parser off of the HelloParser input, sorry
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not ha
Github user mmiklavc commented on the issue:
https://github.com/apache/metron/pull/530
```
cat ~/.m2/repository/archetype-catalog.xml
http://maven.apache.org/plugins/maven-archetype-plugin/archetype-catalog/1.0.0
http://maven.apache.org/xsd/archetype-catalog-1.0.0.xsd";
Github user mmiklavc commented on the issue:
https://github.com/apache/metron/pull/530
```
# * 04727ca8c 2017-08-03 | update document to remove references to the
working dir (HEAD, ottobackwards/METRON-777) [Otto Fowler]
metron-deployment/scripts/platform-info.sh
Metro
Github user ottobackwards commented on the issue:
https://github.com/apache/metron/pull/530
I DO think that the archetype's input variables could use refactoring. For
example, I force the artifact name, and maybe I should not.
I do not know why the bundle plugin is looking fo
Github user ottobackwards commented on the issue:
https://github.com/apache/metron/pull/530
```bash
ââ[ottofowler@Winterfell] -
[~/.m2/repository/org/apache/metron/metron-maven-parser-extension-archetype/0.4.1]
- [Thu Aug 03, 18:42]
ââ[$]> ll
total 80
-rw-r--r-
Github user ottobackwards commented on the issue:
https://github.com/apache/metron/pull/530
```bash
ââ[ottofowler@Winterfell] - [~/tmp] - [Thu Aug 03, 18:36]
ââ[$]> mvn archetype:generate -DarchetypeCatalog=local
[INFO] Scanning for projects...
[INFO]
[I
Github user mmiklavc commented on the issue:
https://github.com/apache/metron/pull/530
Hrm, this is what I got this time around, changing version to 0.4.1:
```
5: local -> org.apache.metron:metron-maven-parser-extension-archetype
(Apache Maven Parser Extension Archetype for Met
Github user ottobackwards commented on the issue:
https://github.com/apache/metron/pull/530
I'm not done the build, so I'll try this again, but this is what I get: (
Note, I don't know how you are getting the packageInPathFormat option ):
ââ[ottofowler@Winterfell] - [~/tm
Github user ottobackwards commented on the issue:
https://github.com/apache/metron/pull/530
WRT: the archetype directories: I don't see that ( but I'm going to
re-build and try again ).
I don't use the same options you are using though.
Importantly -> the parser version M
Github user mmiklavc commented on the issue:
https://github.com/apache/metron/pull/530
Ok, I see what that's doing now re: metron-parsers. As I was glancing over
the project dirs my next question was going to be about what is in
metron-parsers vs the extensions, but I follow. I do bel
Github user mmiklavc commented on the issue:
https://github.com/apache/metron/pull/530
Will review your other comments shortly. Some additional questions:
I ran the archetype to create a new system parser:
```
[INFO] Parameter: groupId, Value: com.michaelmiklavcic
[INFO]
Github user ottobackwards commented on the issue:
https://github.com/apache/metron/pull/530
Ok - I think you are running find from /usr/metron/$V/
So - to explain the other config/zookeeper
So let's pretend there are two things here:
1. metron-parsers
2. parser-exte
Github user ottobackwards commented on the issue:
https://github.com/apache/metron/pull/530
This comes through better in METRON-942, where an extension built with the
archetype is actually uploaded and installed into metron - ready to be
activated. This is a much better flow and expe
Github user ottobackwards commented on the issue:
https://github.com/apache/metron/pull/530
thanks @mmiklavc. I don't think I've stated this in the actual PR
As a 3rd party ( or whatever you want to call it ) developer, who only
makes Parser or possibly other extensions f
Github user mmiklavc commented on the issue:
https://github.com/apache/metron/pull/530
I see that for parser extensions, the enrichment and indexing
configurations have also been moved into this tree. This structure conflates
parsers, enrichments, and indexing. Beyond that, I also see
Github user ottobackwards commented on the issue:
https://github.com/apache/metron/pull/530
Oh profiler integration test
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled a
Github user ottobackwards commented on the issue:
https://github.com/apache/metron/pull/530
@mmiklavc
1. I don't think we want to, or have to track it as upstream. But we do
want to keep track of bug fixes and improvements to areas where we have not
deviated. That is my curren
Github user mmiklavc commented on the issue:
https://github.com/apache/metron/pull/530
I'm back to looking through this again. A couple questions:
1) I was under the impression that our local copy of the nar functionality
would be a hard fork that would naturally deviate from the N
Github user ottobackwards commented on the issue:
https://github.com/apache/metron/pull/530
"Based on feedback from reviews and discussion with mattf, this commitâ¦
⦠represent a major refactoring for the bundle functionality.
While trying to resolve the ugliness bought
Github user ottobackwards commented on the issue:
https://github.com/apache/metron/pull/530
METRON-1070
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or
Github user mattf-horton commented on the issue:
https://github.com/apache/metron/pull/530
and thanks for the file map of the test area.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this f
Github user mattf-horton commented on the issue:
https://github.com/apache/metron/pull/530
@ottobackwards , I agree it isn't critical to update now, and would slow
down this PR even more. Do you mind opening a tracking jira to consider an
update in the future? (assuming they continu
Github user ottobackwards commented on the issue:
https://github.com/apache/metron/pull/530
@mattf-horton I have reviewed the changes between what I have and 1.3.0,
and while there are some one or two slight differences, there is nothing there
that functionally improves or applies to
Github user ottobackwards commented on the issue:
https://github.com/apache/metron/pull/530
@mattf-horton I think I have address the issues up until now. I have not
done the compare to nifi 1.3.0 to see the difference however.
---
If your project is set up for it, you can reply to t
Github user ottobackwards commented on the issue:
https://github.com/apache/metron/pull/530
```bash
.
âââ java
â  âââ org
â  âââ apache
â  âââ metron
â  âââ bundles
â Â
Github user ottobackwards commented on the issue:
https://github.com/apache/metron/pull/530
No, there is no problem with you referring back to nifi @simonellistonball
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If yo
Github user simonellistonball commented on the issue:
https://github.com/apache/metron/pull/530
@ottobackwards I didn't steal _that_ much of it. Still it shouldn't cause a
problem, as long as we get all the updates since this PR started ported in.
That one has moved a bit from things
Github user ottobackwards commented on the issue:
https://github.com/apache/metron/pull/530
That CEFParserTest is actually from someone else's 'appropriation' of Nifi
code ;)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as wel
Github user mattf-horton commented on the issue:
https://github.com/apache/metron/pull/530
@ottobackwards , can you please give me a similar file mapping for the
"test" side of bundles-lib, ie bundles-lib/src/test/java/org/apache/metron/ vs
nifi/nifi-nar-bundles/nifi-framework-bundle/
Github user mattf-horton commented on the issue:
https://github.com/apache/metron/pull/530
Interesting! Thanks for checking @ottobackwards .
In metron-platform/metron-extensions, possibly incorrect usage of nar,
which should -> bundle:
```
metron-platform/metron-exte
Github user ottobackwards commented on the issue:
https://github.com/apache/metron/pull/530
WRT: Required Bundle dependency for BundleProvidedDependenciesMojo.
That mojo is not run automatically, either in metron or in the nifi case.
It is *explicitly* run to determine the de
Github user mattf-horton commented on the issue:
https://github.com/apache/metron/pull/530
In bundles-lib/, possibly incorrect usages of nar, which should -> bundle:
```
bundles-lib//src/main/java/org/apache/metron/bundles/bundle/BundleDetails.java:
* Builder for NarDetail
Github user ottobackwards commented on the issue:
https://github.com/apache/metron/pull/530
I'm actually going to debug this and verify about the dependency
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project
Github user ottobackwards commented on the issue:
https://github.com/apache/metron/pull/530
> My question was, since it throws an exception if no Bundle dependency is
stated, if I have a Bundle that is not actually dependent on any other bundles,
what should I declare it is dependent
Github user mattf-horton commented on the issue:
https://github.com/apache/metron/pull/530
### Code tree differences in bundles-lib vs nifi-nar-utils
Thanks for the analysis, this will help me a lot.
### Bundle Null Dependency question
> So, since everything in nifi is
Github user ottobackwards commented on the issue:
https://github.com/apache/metron/pull/530
WRT: Maven Plugin versions : There are no changes that are not my changes,
so we have the 1.2.0 release code
---
If your project is set up for it, you can reply to this email and have your
rep
Github user ottobackwards commented on the issue:
https://github.com/apache/metron/pull/530
```code
-
To have a single lib, some things that were in other nifi libraries had to
be relocated.
./bundles:
ExtensionManager.java-> MODIFIED
Nar
1 - 100 of 183 matches
Mail list logo
