METRON-1212 The bundle system and maven plugin #774
Conversation
|
@bbende |
|
FlatFile tests are failing with timeouts, kicking |
|
@ottobackwards Are we having intermittent test failures again? |
|
On that day the Flat File test failed. Since it worked after open/close I wrote it off to a load / timing issue. |
|
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 ) |
I have just started into this, but seems like I can create a bundle using the Maven plugin with this PR, right? |
|
yes you can |
Testing the bundle pluginI have created a sample project that can be examined for testing the plugin: |
|
@nickwallen are you still looking at this? |
|
No, I need to get back to this. Are you? Don't let me stop your own review. |
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 bundle? Is there anything else that should be part of the test plan? |
|
@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. |
|
Sounds good @JonZeolla thanks |
@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? |
|
Thanks for looking at this again Nick, let me try to answer your questions. TL;DRI'm sorry for my confusion with your original statement. I had in mind this part of @mattf-horton 's email on these pull requests:
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. |
|
I will extend the readme |
|
@nickwallen hopefully the Readme changes help. |
|
@ottobackwards says:
{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:
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: #530 (comment) , 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,
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. |
The Getting Started information that you added to the README is outstanding! |
|
@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. |
|
I checkout your PR and then attempt to do an install and the build fails. I noticed that you changed the .travis build instructions so that you have to manually go into the 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? |
|
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. |
| * Add a new module for your bundle, it needs only to have a pom.xml | ||
| * Create the pom.xml as above, with the correct plugin and packaging entries, and add dependencies | ||
| for the module you want to bundle. | ||
| * `mvn package` |
There was a problem hiding this comment.
Overall to test your PR, I want to do the following...
- Create a bundle
- Write some custom code in that bundle
- Deploy the bundle somehow and see the code in that bundle execute
I am now trying to complete step 1 and just create a bundle. Isn't there a mvn archetype:generate command that I have to run to create a bundle? Where are the docs outlining those steps? I would have expected to see them here, I think, but I don't
There was a problem hiding this comment.
The getting started section of the plugin documentation tells you how to create a bundle.
The bundles-testing module is a simple result of following those instructions, with an added integration module with an example of using a bundle in code like you are looking to do.
The archetype is for creating specific types of projects that produce artifacts that may include a bundle. For example an archetype for creating a parser extension that produces an assembly with a bundle in it.
There was a problem hiding this comment.
I should say the getting started and the quick-start
There was a problem hiding this comment.
Can you point me to a specific file and section? I can't find instructions in the 3 new READMEs that I see in the PR. Maybe I am just missing it somehow.
There was a problem hiding this comment.
Are you looking for instructions on how to create the bundles-testing project step by step?
like : create a folder. in that folder create 4 folders. create a file pom.xml.... ?
There was a problem hiding this comment.
I wish you would have stated your disagreement during the community meeting, or in the email list on the 26th when you gave a +1 to this.
It is unfortunate that we again have gone months on a PR in this now year long effort to no effect, especially since as stated this was supposed to be the 'lighter' review of all the pr's.
That being said, we should not be discouraging reviews under any circumstances, and your review certainly matters.
So please. Can you explain what would would like to do based on your current understanding now that I have attempted to clarify things, and tried to describe the projects that are present?
There was a problem hiding this comment.
Nifi does not have this documented, and does not have a 'generic' archetype. @nickwallen I wonder if an archetype that just creates a simple bundle project would be useful? Would that be more of what you are looking for than the bundles-testing? Or would a written guide be the thing? Does this guide have to document how to create the multi module project? Does it have to document both existing and from scratch scenarios?
There was a problem hiding this comment.
Let me clarify, when I say that nifi doesn't have this documented, I mean that their documentation, as mine ( though not included in this PR ) revolves around creating NARs of NiFi components, and not generic NARs.
There was a problem hiding this comment.
Sorry @ottobackwards. Not ignoring you. I have just been busy. Will get back to this soon. Thanks for your hard work in helping me through this.
There was a problem hiding this comment.
no problem, let me know if the new doc is in line with what you are looking for
|
Bringing over this from #530, apache/nifi@d90cf84#diff-83e1afb34470ca47809f82aa1caf2138 is the commit in nifi for the nar-utils to diff. |
|
@nickwallen I have added a usage document in the last commit. Is this what you are looking for? |
|
I have refactored to have a top level metron-bundles directory, with the bundles lib and bundles testing under there. |
as compared to the feature branch the differences are: - Added some modules to test loading a bundle, and executing a function - checkstyle changes - some minor typo fixes
Clean up the bundles-testing
ottobackwards
force-pushed
the
fifth_bundles
branch
from
e5f89e1 to
2bbd3f7
Compare
|
I have created a new feature branch feature/METRON-1211-extensions-parsers-gradual to track this. I have also updated confluence and jira : https://cwiki.apache.org/confluence/display/METRON/Metron+Extension+System+and+Parser+Extensions
Will email list |
|
The new Trusty image breaks my build. I need to figure out why exactly, so I'll be trying a few things. |
4 participants
This PR contains the Bundle system and Maven Plugin.
The bundle system and the plugin are adapted from the Apache Nifi project.
bundles-maven-plugin
The bundles-maven-plugin is an adapted version of the jar dependency plugin whose function is to bundle a jar of jars based on the dependencies for a project. It also creates metadata attributes.
A project's jar, and it's non-provided dependency jars are place in a /lib entry in the bundle, with the bundle itself being in jar format.
bundles-lib
The bundles-lib contains the functionality required to:
NAR exposed the bundles through many classes. I have created the BundleSystem interface to expose a more usable, simplified api for our use cases.
From the original PR for METRON-777:
Metron Bundle Plugin
bundle-lib
Previous Review
Please see @mattf_apache's review
changes from that review
I have changed the InitContext operations to have explicit builders, and made it so that creating a context can be done separately from initialization. Two contexts can then be 'merged'. This is to allow for the addition of new bundles after initialization.
In preparing this PR I have:
Testing
*
cd bundles-maven-plugin && mvn -q install && cd ..must be run once to install the maven pluginFor all changes:
For documentation related changes: