[GitHub] metron issue #774: METRON-1212 The bundle system and maven plugin
Github user ottobackwards commented on the issue: https://github.com/apache/metron/pull/774 The new Trusty image breaks my build. I need to figure out why exactly, so I'll be trying a few things. ---
[GitHub] metron issue #774: METRON-1212 The bundle system and maven plugin
Github user ottobackwards commented on the issue: https://github.com/apache/metron/pull/774 I have created a new feature branch feature/METRON-1211-extensions-parsers-gradual to track this. and have rebased this PR on to that. I have also updated confluence and jira : https://cwiki.apache.org/confluence/display/METRON/Metron+Extension+System+and+Parser+Extensions 1. Feature Branch still makes sense 2. now that we are splitting it up it will work better 3. I don't want to have to worry about regression for short periods Will email list ---
[GitHub] metron issue #774: METRON-1212 The bundle system and maven plugin
Github user ottobackwards commented on the issue: https://github.com/apache/metron/pull/774 I have refactored to have a top level metron-bundles directory, with the bundles lib and bundles testing under there. ---
[GitHub] metron issue #774: METRON-1212 The bundle system and maven plugin
Github user ottobackwards commented on the issue: https://github.com/apache/metron/pull/774 @nickwallen I have added a usage document in the last commit. Is this what you are looking for? ---
[GitHub] metron issue #774: METRON-1212 The bundle system and maven plugin
Github user ottobackwards commented on the issue: https://github.com/apache/metron/pull/774 Bringing over this from #530, https://github.com/apache/nifi/commit/d90cf846b9dcd49acdeac7bb5fcd13ee80dcdba6#diff-83e1afb34470ca47809f82aa1caf2138 is the commit in nifi for the nat-utils to diff. ---
[GitHub] metron issue #774: METRON-1212 The bundle system and maven plugin
Github user ottobackwards commented on the issue: https://github.com/apache/metron/pull/774 I'm sorry nick, the instruction for 'how to build metron' from the top level readme didn't get pulled over when I did the reconstruction. The plugin is not in any repo for maven to install from. I don't know how to get it installed since we don't publish it. I will check my list history, but I think we (not you necessarily) talked about doing it this way until we figured out if we were going to publish the plugin. ---
[GitHub] metron issue #774: METRON-1212 The bundle system and maven plugin
Github user nickwallen commented on the issue: https://github.com/apache/metron/pull/774 I checkout your PR and then attempt to do an install and the build fails. ``` $ mvn clean install [INFO] Scanning for projects... Downloading: https://repo.maven.apache.org/maven2/org/apache/metron/bundles-maven-plugin/0.4.2/bundles-maven-plugin-0.4.2.pom [WARNING] The POM for org.apache.metron:bundles-maven-plugin:jar:0.4.2 is missing, no dependency information available Downloading: https://repo.maven.apache.org/maven2/org/apache/metron/bundles-maven-plugin/0.4.2/bundles-maven-plugin-0.4.2.jar [ERROR] [ERROR] Some problems were encountered while processing the POMs: [ERROR] Unresolveable build extension: Plugin org.apache.metron:bundles-maven-plugin:0.4.2 or one of its dependencies could not be resolved: Could not find artifact org.apache.metron:bundles-maven-plugin:jar:0.4.2 in central (https://repo.maven.apache.org/maven2) @ @ [ERROR] The build could not read 1 project -> [Help 1] [ERROR] [ERROR] The project org.apache.metron:bundles-testing:0.4.2 (/Users/nallen/tmp/metron-pr774/bundles-testing/pom.xml) has 1 error [ERROR] Unresolveable build extension: Plugin org.apache.metron:bundles-maven-plugin:0.4.2 or one of its dependencies could not be resolved: Could not find artifact org.apache.metron:bundles-maven-plugin:jar:0.4.2 in central (https://repo.maven.apache.org/maven2) -> [Help 2] [ERROR] [ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch. [ERROR] Re-run Maven using the -X switch to enable full debug logging. [ERROR] [ERROR] For more information about the errors and possible solutions, please read the following articles: [ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/ProjectBuildingException [ERROR] [Help 2] http://cwiki.apache.org/confluence/display/MAVEN/PluginManagerException ``` I noticed that you changed the .travis build instructions so that you have to manually go into the `bundles-maven-plugin` project and install that first. ``` $ git diff origin/master -- .travis.yml diff --git a/.travis.yml b/.travis.yml index f5edfb27..8023830a 100644 --- a/.travis.yml +++ b/.travis.yml @@ -33,6 +33,7 @@ before_install: - npm config set prefix $HOME/.npm-prefix --global install: + - cd bundles-maven-plugin && mvn -q install && cd .. - time mvn -q -T 2C -DskipTests clean install script: ``` Ideally we need to be able to just build Metron by running an install from the root directory. Is there no way around this? What troubles did you run into that caused you to have to do this? ---
[GitHub] metron issue #774: METRON-1212 The bundle system and maven plugin
Github user ottobackwards commented on the issue: https://github.com/apache/metron/pull/774 @mattf-horton I have added some more documentation, and I think the bundles-testing project provides a simple, integrated example. @nickwallen and I talked it over, and he is going to try that. ---
[GitHub] metron issue #774: METRON-1212 The bundle system and maven plugin
Github user nickwallen commented on the issue: https://github.com/apache/metron/pull/774 > @ottobackwards hopefully the Readme changes help. The Getting Started information that you added to the README is outstanding! ---
[GitHub] metron issue #774: METRON-1212 The bundle system and maven plugin
Github user mattf-horton commented on the issue:
https://github.com/apache/metron/pull/774
@ottobackwards says:
> would it be possible for you to help us with a comment as to your
approach when you reviewed this functionality? It might help if you could
summarize your feelings as well
{with music}
Oh spelunking we shall go, spelunking we shall go... :-)
{with music}
While reviewing the original incarnation of this, I tried to focus on the
part most other Metron community members wouldn't be familiar with, the NiFi
Bundle implementation, which I have some (small) experience with due to past
work with NiFi. The review I did was an eyeball line-by-line code review. Of
course actually running the code is also necessary, and at the time @mmiklavc
was focusing on that side of things.
Knowing that (a) the NiFi feature works in NiFi, and (b) Otto's intent was
to change it as little as possible while porting it into Metron, I took an
approach to reviewing it that focused on the **changes.** Specifically, as
best I remember, I applied the following logic:
* I pulled a copy of the version of NiFi that Otto used as a base
* I pulled a copy of Otto's dev branch (submitted in PR#530)
* I observed relationships between directory paths of corresponding files
(code organization relationships) in the two code sets, including filename
changes (which were mostly pattern-based)
* I applied "diff -w" to corresponding file pairs
* I reviewed THAT as the code deltas of interest, the way we would code
review a simple PR based on master
* I also reviewed in detail the small number of newly created files, and
missing/deleted files, compared to the related NiFi sources
* I mapped my questions and remarks back to the submitted code in Otto's
PR#530, and made the usual comments in that PR
* We interacted and followed up as usual.
After many days and some untold number of comments, responses, and changes,
I felt I understood and agreed with the state of the code, and gave it a +1
here: https://github.com/apache/metron/pull/530#issuecomment-322970038 ,
contingent on the practical trials being completed.
Now, we've changed gears a bit. Faced with the difficulty of trying to
review the offering all at once, whether in a PR or in a "feature branch"
(turned out that wasn't much help after all), @ottobackwards and I proposed,
and you'all accepted, the idea of breaking it up into five or so chunks. This
was specified in my email to dev@metron.apache.org, 25 Sep 2017, Subject: Re:
feature branch bumps (see
http://mail-archives.apache.org/mod_mbox/metron-dev/201709.mbox/browser ,
Thread view, page 2, about 3/4 way down the page).
It was specifically called out that for the first chunk, consisting of just
the âbundleâ mechanism and the maven plugin,
> "Review is acknowledged to be purely theoretical, and testing is based on
the junit tests and integration tests. Weâll add a simple integration test
with a synthetic test jar containing a trivial
Class, unrelated to parsers, invoked in a test case via reflection (hence
needing no glue
code). After passing that level of review and testing, it would be
committed, with an understanding that additional revisions might be required as
the result of subsequent PR comments.
So on the one hand, we agreed to a fairly light-weight review process,
since by the nature of a piece-wise submission, the first part can't be tested
on parsers. I encourage you to stick with that resolve.
On the other hand, the simple integration test suggested does call for
being able to see a "hello-world" Stellar command loaded and run from a bundle,
which actually would be kinda exciting.
@ottobackwards , can your test bundle be turned into a Stellar function,
loaded, and invoked (by reflection so you don't need to worry about
registration)? That would look more like an integration test than that bundle
hanging out by itself.
---
[GitHub] metron issue #774: METRON-1212 The bundle system and maven plugin
Github user ottobackwards commented on the issue: https://github.com/apache/metron/pull/774 @nickwallen hopefully the Readme changes help. ---
[GitHub] metron issue #774: METRON-1212 The bundle system and maven plugin
Github user ottobackwards commented on the issue: https://github.com/apache/metron/pull/774 I will extend the readme ---
[GitHub] metron issue #774: METRON-1212 The bundle system and maven plugin
Github user ottobackwards commented on the issue: https://github.com/apache/metron/pull/774 @nickwallen If is OK with you, i'll make it a separate file to the README.md ---
[GitHub] metron issue #774: METRON-1212 The bundle system and maven plugin
Github user ottobackwards commented on the issue: https://github.com/apache/metron/pull/774 Thanks for looking at this again Nick, let me try to answer your questions. TL;DR I'm sorry for my confusion with your original statement. I had in mind this part of @mattf-horton 's email on these pull requests: > The first PR will be just the âbundleâ mechanism and the maven plugin. > Both are adaptations from the Apache NiFi project, and have already been > reviewed as part of PR# 530. Review is acknowledged to be purely > theoretical, and testing is based on the junit tests and integration > tests. I thought an example project to look at would answer your question, not that you actually wanted to do it, or have instructions to do it. Especial since you didn't say "the readme should have better instructions for creating a project that uses the plugin". While creating bundles by hand is not in the initial use case, if you feel it is important I'll write a readme. - As with the NiFi version of the maven plugin, the 100% use case in practice ( and for initial dev in metron ) is to have bundles create as part of the archetype project generation. When I 'adapted' the maven plugin, which consisted of adding the ability to rename the extension of the product and change the metadata prefixes, I did not make changes to the documentation of the plugin beyond documenting my changes. I also did not attempt to add maven test harness support for the plugin, as it was not present in the original project. Thus, the create by hand scenario for our version is as lacking as the create by hand scenario is in the origin nifi project. I tried to avoid changing the plugin as much as possible, as it is already in use in NiFi. As we had discussed in the threads and the meetings, this initial PR would contain the plugin and the bundle, and the practical testing would be limited. I created and added reference to the test project because it is a simple example of a project that utilizes the plugin to create a bundle. I did this as opposed to writing out a step by step guide to how to create a multi module maven project, as I mistakenly thought this would be helpful given the context of the review. @mattf-horton, would it be possible for you to help us with a comment as to your approach when you reviewed this functionality? It might help if you could summarize your feelings as well. The context is different, as the review within 777 could not be a +1 for the PR since this was only part of the total, but since you are the only one to have had reviewed this, your views are pretty important. ---
[GitHub] metron issue #774: METRON-1212 The bundle system and maven plugin
Github user nickwallen commented on the issue: https://github.com/apache/metron/pull/774 > Test Project can be examined as a simple example of how to create bundles. @ottobackwards Maybe I'm being really stupid, but I don't understand how the test-bundles-plugin shows me how to create bundles. Wouldn't I use the code in this PR to create something like the test-bundles-plugin? Does that make sense? What command do I run to create a bundle? How do I validate that the bundle created is valid? ---
[GitHub] metron issue #774: METRON-1212 The bundle system and maven plugin
Github user nickwallen commented on the issue: https://github.com/apache/metron/pull/774 Sounds good @JonZeolla thanks ---
[GitHub] metron issue #774: METRON-1212 The bundle system and maven plugin
Github user JonZeolla commented on the issue: https://github.com/apache/metron/pull/774 @nickwallen I am doing some functional testing, but I'm not comfortable enough with it to give a full +1 on my own. When I'm done I will report back. ---
[GitHub] metron issue #774: METRON-1212 The bundle system and maven plugin
Github user nickwallen commented on the issue: https://github.com/apache/metron/pull/774 > @nickwallen: I have just started into this, but seems like I can create a bundle using the Maven plugin with this PR, right? > @ottobackwards: yes you can This was more of a leading question. Can we add this step to the test plan? Can you point to the directions on creating a bungle? Is there anything else that should be part of the test plan? ---
[GitHub] metron issue #774: METRON-1212 The bundle system and maven plugin
Github user JonZeolla commented on the issue: https://github.com/apache/metron/pull/774 @nickwallen are you still looking at this? ---
[GitHub] metron issue #774: METRON-1212 The bundle system and maven plugin
Github user ottobackwards commented on the issue: https://github.com/apache/metron/pull/774 # Testing the bundle plugin I have created a sample project that can be examined for testing the plugin: [test-bundles-plugin](https://github.com/ottobackwards/test-bundles-plugin) ---
[GitHub] metron issue #774: METRON-1212 The bundle system and maven plugin
Github user ottobackwards commented on the issue: https://github.com/apache/metron/pull/774 yes you can ---
[GitHub] metron issue #774: METRON-1212 The bundle system and maven plugin
Github user nickwallen commented on the issue: https://github.com/apache/metron/pull/774 > This review is code review and test code review and running only I have just started into this, but seems like I can create a bundle using the Maven plugin with this PR, right? ---
[GitHub] metron issue #774: METRON-1212 The bundle system and maven plugin
Github user ottobackwards commented on the issue: https://github.com/apache/metron/pull/774 From a quick check of other PR travis failures, it looks like my build was the only one to have that problem ( which as an expected 1000 got 908 issue ) ---
[GitHub] metron issue #774: METRON-1212 The bundle system and maven plugin
Github user ottobackwards commented on the issue: https://github.com/apache/metron/pull/774 On that day the Flat File test failed. Since it worked after open/close I wrote it off to a load / timing issue. https://travis-ci.org/apache/metron/builds/280393026 ---
[GitHub] metron issue #774: METRON-1212 The bundle system and maven plugin
Github user nickwallen commented on the issue: https://github.com/apache/metron/pull/774 @ottobackwards Are we have intermittent test failures again? ---
[GitHub] metron issue #774: METRON-1212 The bundle system and maven plugin
Github user ottobackwards commented on the issue: https://github.com/apache/metron/pull/774 FlatFile tests are failing with timeouts, kicking ---
[GitHub] metron issue #774: METRON-1212 The bundle system and maven plugin
Github user ottobackwards commented on the issue: https://github.com/apache/metron/pull/774 @bbende this is going in as a separate PR now. I am not sure if you are interest or not, but I figure I'll give you a heads up. ---
