You are viewing a plain text version of this content. The canonical link for it is here.

Posted to dev@metron.apache.org by mattf-horton <gi...@git.apache.org> on 2017/06/10 01:18:22 UTC

[GitHub] metron pull request #616: METRON-877 Extract core implementation and UDF sup...

GitHub user mattf-horton opened a pull request:

    https://github.com/apache/metron/pull/616

    METRON-877 Extract core implementation and UDF support, create metron-stellar module

    ## Contributor Comments
    It appears that github permanently closes any PR that gets rebased, hence for the history and discussion to date, I can only point you at the previous incarnation of this PR: https://github.com/apache/metron/pull/610
    
    The design document has been available for some time at https://docs.google.com/document/d/1EP7Jt4ePHe2A-_oboLl2QbN1muh7uKeET_kbpIgjcJM/edit#
    I have now added a new section, starting on p. 7, titled "Retrospective: Changes made for Parts 1 & 2" that details the specific changes actually done in this PR to extract Stellar.
    
    ## Pull Request Checklist
    In order to streamline the review of the contribution we ask you follow these guidelines and ask you to double check the following:
    
    ### For all changes:
    - [x] Is there a JIRA ticket associated with this PR? If not one needs to be created at [Metron Jira](https://issues.apache.org/jira/browse/METRON/?selectedTab=com.atlassian.jira.jira-projects-plugin:summary-panel). 
    - [x] Does your PR title start with METRON-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.
    - [x] Has your PR been rebased against the latest commit within the target branch (typically master)?
    
    ### For code changes:
    The intent of these changes is to achieve an architecture change WITHOUT any functional change.  The continued working of all unit tests, integration tests and real-world applications should result.  In addition, it generates (in metron-stellar/stellar-common/target) a stellar-common uber-jar (including all dependencies), which among other things can run the Stellar shell as a stand-alone Java application (although missing those Stellar functions that remain as part of Metron):
    ```
    java -cp target/stellar-common-0.4.0-uber.jar \
        org.apache.metron.stellar.common.shell.StellarShell
    ```
    For testing within Metron, the stellar classes are still incorporated into the metron-common jar and RPM, so test installation should work as before.
    
    For a general test plan for all Stellar functionality within Metron, please see #468 's test plan. 
    
    
    - [x] Have you included steps to reproduce the behavior or problem that is being changed or addressed?
    - [x] Have you included steps or a guide to how the change may be verified and tested manually?
    - [x] Have you ensured that the full suite of tests and checks have been executed in the root incubating-metron folder via:
      ```
      mvn -q clean integration-test install && build_utils/verify_licenses.sh 
      ```
    
    - [x] Have you written or updated unit tests and or integration tests to verify your changes?
    - [x] 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)? 
    - [IN PROCESS] Have you verified the basic functionality of the build by building and running locally with Vagrant full-dev environment or the equivalent?
    
    ### For documentation related changes:
    - [x] Have you ensured that format looks appropriate for the output in which it is rendered by building and verifying the site-book? 


You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/mattf-horton/metron stellar-mod4

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/metron/pull/616.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 #616
    
----
commit 9927da1f24b23985c129c5996e92292c14270fb9
Author: mattf-horton <mf...@hortonworks.com>
Date:   2017-06-05T22:15:15Z

    Stellar extraction - first commit, steps 1 and 2.

commit d00588f03ab05625ced694cda0621a78694adeed
Author: mattf-horton <mf...@hortonworks.com>
Date:   2017-06-07T06:45:04Z

    corrections for build_utils/verify_licenses.sh

commit ed78af4bf4a0e9ae4c88c1326531b6d61bc18ecc
Author: mattf-horton <mf...@hortonworks.com>
Date:   2017-06-08T01:20:52Z

    fix for EnrichmentConfigFunctionsTest issue

commit 8ce3d8a52f5758c740d0dd4664cdbe9969b2be95
Author: mattf-horton <mf...@hortonworks.com>
Date:   2017-06-09T21:18:21Z

    fix RPM generation after stellar separation.  Stellar-common continues to be built into metron-common uber-jar.

commit 1349dcc85b75b621922c8c01fcbe6ba3d569ce1e
Author: mattf-horton <mf...@hortonworks.com>
Date:   2017-06-10T00:45:01Z

    cleanup after rebase to current master

----


---
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 if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] metron issue #616: METRON-877 Extract core implementation and UDF support, c...

Posted by cestella <gi...@git.apache.org>.

Github user cestella commented on the issue:

    https://github.com/apache/metron/pull/616
  
    Ah, yes, I do want that.


---
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 if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] metron issue #616: METRON-877 Extract core implementation and UDF support, c...

Posted by ottobackwards <gi...@git.apache.org>.

Github user ottobackwards commented on the issue:

    https://github.com/apache/metron/pull/616
  
    so can this be merged?



---
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 if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] metron issue #616: METRON-877 Extract core implementation and UDF support, c...

Posted by ottobackwards <gi...@git.apache.org>.

Github user ottobackwards commented on the issue:

    https://github.com/apache/metron/pull/616
  
    So we have a StatefulExecutor, is there an idea that there will be more types of executors?  Will there be a non-stateful executor, maybe for performance reasons or other use cases?  Maybe in the future some write up on this would help with the __intent__


---
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 if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] metron issue #616: METRON-877 Extract core implementation and UDF support, c...

Posted by cestella <gi...@git.apache.org>.

Github user cestella commented on the issue:

    https://github.com/apache/metron/pull/616
  
    +1 by inspection, this is great work, @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 wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] metron issue #616: METRON-877 Extract core implementation and UDF support, c...

Posted by mattf-horton <gi...@git.apache.org>.

Github user mattf-horton commented on the issue:

    https://github.com/apache/metron/pull/616
  
    Thanks, @ottobackwards and @cestella !
    Merge to current master was trivial, as seen here: https://github.com/apache/metron/pull/616/commits/87a44f600678ffe246ac7da49128e0ac8c3cf8c1
    Proceeding with commit.


---
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 if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] metron issue #616: METRON-877 Extract core implementation and UDF support, c...

Posted by ottobackwards <gi...@git.apache.org>.

Github user ottobackwards commented on the issue:

    https://github.com/apache/metron/pull/616
  
    As a more stand alone module, I think the readme should be broken down a little bit, instead of just being 'here is the language'.
    
    A top level statement document, with what stellar is at a high level, why it exists and when it has value to be used.
    Below that more technical guides - language, examples, faq, discovery and loading
    
    I wouldn't hold this PR up for this, but maybe it should be added to the follow on list.
    



---
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 if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] metron issue #616: METRON-877 Extract core implementation and UDF support, c...

Posted by mattf-horton <gi...@git.apache.org>.

Github user mattf-horton commented on the issue:

    https://github.com/apache/metron/pull/616
  
    @ottobackwards , thanks very much for tackling this admittedly large review.
    
    > So, the test plan for this is...?
    
    "For a general test plan for all Stellar functionality within Metron, please see #468 's test plan."
    It's long, hence not reproduced here.
    
    > So we have a StatefulExecutor...
    
    The base executor is org.apache.metron.stellar.common.BaseStellarProcessor, formerly org.apache.metron.common.stellar.BaseStellarProcessor.  It parses and evaluates Stellar expressions, with caching of the parse, and with given context (variables), variableResolver, and functionResolver.  It can return any Stellar data type.  It also provides a validate() method.
    
    The "stateful" executor, org.apache.metron.stellar.common.StellarStatefulExecutor, assists with Aggregators, where state needs to persist across multiple invocations of the same entity, and in fact was formerly located at org.apache.metron.profiler.stellar.StellarExecutor.  It was renamed when I relocated it, to avoid conflict with the Shell executor, org.apache.metron.stellar.common.shell.StellarExecutor, which is significantly different.  I deemed the stateful executor to be Stellar functionality rather than Profiler, and moved it.  The Aggregator functions will also be moved into Stellar, but that will be in Step 3, which will be a different PR.
    
    I agree in future it would be nice to have design documents, for Stellar and other parts of Metron.
    
    > I think the readme should be \[expanded into a Language Reference Manual :-) \]
    > ...maybe it should be added to the follow on list.
    
    Yah, that would be good.  I've opened [METRON-998](https://issues.apache.org/jira/browse/METRON-998), as a follow-on task.
    
    > I think that this could be a feature branch.
    
    Since this started as a regular PR, I'd like to finish this one that way.  But I agree, before starting Part 3 I'll petition to create a feature branch for the next set(s) of work.



---
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 if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] metron issue #616: METRON-877 Extract core implementation and UDF support, c...

Posted by ottobackwards <gi...@git.apache.org>.

Github user ottobackwards commented on the issue:

    https://github.com/apache/metron/pull/616
  
    Each example could be the seed of future development of encapsulating that functionality.  For example - how to use stellar in spark could be evolved into a stellar-spark integration package.


---
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 if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

Re: [GitHub] metron issue #616: METRON-877 Extract core implementation and UDF support, c...

Posted by Matt Foley <mf...@hortonworks.com>.

Yes, it could cause problems. Not sure what to do about it. Should we open an INFRA ticket, or does our PMC have auth to poke it?

One can browse the (real) Apache repo and see it, it hasn't been pushed from there.

--Matt



On Mon, Jul 3, 2017 at 4:16 AM -0700, "Otto Fowler" <ot...@gmail.com>> wrote:


This still has not hit.  Should we be concerned about merges/commits before
it does?



On July 2, 2017 at 19:27:43, mattf-horton (git@git.apache.org) wrote:

Github user mattf-horton commented on the issue:

https://github.com/apache/metron/pull/616

This has been committed to git-wip-us.apache.org/repos/asf/metron.
Propagation seems a little slow today.



---
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 if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

Re: [GitHub] metron issue #616: METRON-877 Extract core implementation and UDF support, c...

Posted by Otto Fowler <ot...@gmail.com>.

This still has not hit.  Should we be concerned about merges/commits before
it does?



On July 2, 2017 at 19:27:43, mattf-horton (git@git.apache.org) wrote:

Github user mattf-horton commented on the issue:

https://github.com/apache/metron/pull/616

This has been committed to git-wip-us.apache.org/repos/asf/metron.
Propagation seems a little slow today.



---
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 if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] metron issue #616: METRON-877 Extract core implementation and UDF support, c...

Posted by mattf-horton <gi...@git.apache.org>.

Github user mattf-horton commented on the issue:

    https://github.com/apache/metron/pull/616
  
    This has been committed to git-wip-us.apache.org/repos/asf/metron.  Propagation seems a little slow today.



---
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 if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] metron issue #616: METRON-877 Extract core implementation and UDF support, c...

Posted by ottobackwards <gi...@git.apache.org>.

Github user ottobackwards commented on the issue:

    https://github.com/apache/metron/pull/616
  
    + 1 on inspection, built and ran the tests -  This is a lot of work, great job.  As with other large PR's ( cough METRON-777 ) I think that this could be a feature branch.  Although this did have more involvement from other contributors ( @cestella ), the feature branch would help the complete review pool


---
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 if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] metron issue #616: METRON-877 Extract core implementation and UDF support, c...

Posted by ottobackwards <gi...@git.apache.org>.

Github user ottobackwards commented on the issue:

    https://github.com/apache/metron/pull/616
  
    Thanks Matt, I think your write up above should be captured somewhere...  We don't want to lose it in the PR


---
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 if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] metron issue #616: METRON-877 Extract core implementation and UDF support, c...

Posted by ottobackwards <gi...@git.apache.org>.

Github user ottobackwards commented on the issue:

    https://github.com/apache/metron/pull/616
  
    Yes, sorry.
    +1


---
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 if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] metron issue #616: METRON-877 Extract core implementation and UDF support, c...

Posted by mattf-horton <gi...@git.apache.org>.

Github user mattf-horton commented on the issue:

    https://github.com/apache/metron/pull/616
  
    Hi @ottobackwards , I've captured those two paragraphs in a new section of the [GoogleDoc](https://docs.google.com/document/d/1EP7Jt4ePHe2A-_oboLl2QbN1muh7uKeET_kbpIgjcJM/edit#), titled "Design Observations".


---
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 if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] metron issue #616: METRON-877 Extract core implementation and UDF support, c...

Posted by ottobackwards <gi...@git.apache.org>.

Github user ottobackwards commented on the issue:

    https://github.com/apache/metron/pull/616
  
    So, the test plan for this is...?  The test work etc?


---
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 if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---