Details

    • Type: Improvement
    • Status: To Do
    • Priority: Major
    • Resolution: Unresolved
    • Affects Version/s: None
    • Fix Version/s: None
    • Labels:
      None

      Description

      While we can add multiple values in STATS_ADD via STATS_ADD(s, 1, 2, 3), but it'd be nice also to support STATS_ADD(s, [1, 2, 3])

        Issue Links

          Activity

          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user cestella opened a pull request:

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

          METRON-1179: Make STATS_ADD to take a list

            1. Contributor Comments
              While we can add multiple values in `STATS_ADD` via `STATS_ADD(s, 1, 2, 3)`, but it'd be nice also to support `STATS_ADD(s, [1, 2, 3])`
            1. Pull Request Checklist

          Thank you for submitting a contribution to Apache Metron.
          Please refer to our [Development Guidelines](https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=61332235) for the complete guide to follow for contributions.
          Please refer also to our [Build Verification Guidelines](https://cwiki.apache.org/confluence/display/METRON/Verifying+Builds?show-miniview) for complete smoke testing guides.

          In order to streamline the review of the contribution we ask you follow these guidelines and ask you to double check the following:

              1. For all changes:
              1. For code changes:
          • [ ] Have you included steps to reproduce the behavior or problem that is being changed or addressed?
          • [ ] Have you included steps or a guide to how the change may be verified and tested manually?
          • [ ] Have you ensured that the full suite of tests and checks have been executed in the root metron folder via:
            ```
            mvn -q clean integration-test install && build_utils/verify_licenses.sh
            ```
          • [ ] Have you written or updated unit tests and or integration tests to verify your 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)?
          • [ ] Have you verified the basic functionality of the build by building and running locally with Vagrant full-dev environment or the equivalent?
              1. For documentation related changes:
          • [ ] Have you ensured that format looks appropriate for the output in which it is rendered by building and verifying the site-book? If not then run the following commands and the verify changes via `site-book/target/site/index.html`:

          ```
          cd site-book
          mvn site
          ```

                1. 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.
                  It is also recommended that [travis-ci](https://travis-ci.org) is set up for your personal repository such that your branches are built there before submitting a pull request.

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

          $ git pull https://github.com/cestella/incubator-metron stats_add_list

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

          https://github.com/apache/metron/pull/750.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 #750


          commit 53ab5ef225c0ba74f7d58f43e3123a7cc325efe7
          Author: cstella <cestella@gmail.com>
          Date: 2017-09-12T15:04:45Z

          Add STATS_ADD_ALL

          commit 72b93b678d782c5f67eee2fbf75c3ebadf82196a
          Author: cstella <cestella@gmail.com>
          Date: 2017-09-12T15:16:24Z

          last revision.

          commit 55a616e79e1c9feeb894b0442f220192d5192d9e
          Author: cstella <cestella@gmail.com>
          Date: 2017-09-12T15:18:15Z

          Updating


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user cestella opened a pull request: https://github.com/apache/metron/pull/750 METRON-1179 : Make STATS_ADD to take a list Contributor Comments While we can add multiple values in `STATS_ADD` via `STATS_ADD(s, 1, 2, 3)`, but it'd be nice also to support `STATS_ADD(s, [1, 2, 3] )` Pull Request Checklist Thank you for submitting a contribution to Apache Metron. Please refer to our [Development Guidelines] ( https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=61332235 ) for the complete guide to follow for contributions. Please refer also to our [Build Verification Guidelines] ( https://cwiki.apache.org/confluence/display/METRON/Verifying+Builds?show-miniview ) for complete smoke testing guides. 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: [ ] 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 ). [ ] 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. [ ] Has your PR been rebased against the latest commit within the target branch (typically master)? For code changes: [ ] Have you included steps to reproduce the behavior or problem that is being changed or addressed? [ ] Have you included steps or a guide to how the change may be verified and tested manually? [ ] Have you ensured that the full suite of tests and checks have been executed in the root metron folder via: ``` mvn -q clean integration-test install && build_utils/verify_licenses.sh ``` [ ] Have you written or updated unit tests and or integration tests to verify your 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)? [ ] 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: [ ] Have you ensured that format looks appropriate for the output in which it is rendered by building and verifying the site-book? If not then run the following commands and the verify changes via `site-book/target/site/index.html`: ``` cd site-book mvn site ``` 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. It is also recommended that [travis-ci] ( https://travis-ci.org ) is set up for your personal repository such that your branches are built there before submitting a pull request. You can merge this pull request into a Git repository by running: $ git pull https://github.com/cestella/incubator-metron stats_add_list Alternatively you can review and apply these changes as the patch at: https://github.com/apache/metron/pull/750.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 #750 commit 53ab5ef225c0ba74f7d58f43e3123a7cc325efe7 Author: cstella <cestella@gmail.com> Date: 2017-09-12T15:04:45Z Add STATS_ADD_ALL commit 72b93b678d782c5f67eee2fbf75c3ebadf82196a Author: cstella <cestella@gmail.com> Date: 2017-09-12T15:16:24Z last revision. commit 55a616e79e1c9feeb894b0442f220192d5192d9e Author: cstella <cestella@gmail.com> Date: 2017-09-12T15:18:15Z Updating
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user ottobackwards commented on a diff in the pull request:

          https://github.com/apache/metron/pull/750#discussion_r138506017

          — Diff: metron-analytics/metron-statistics/src/main/java/org/apache/metron/statistics/StellarStatisticsFunctions.java —
          @@ -141,8 +141,21 @@ public Object apply(List<Object> args) {

          // add each of the numeric values
          for(int i=1; i<args.size(); i++) {

          • double value = convert(args.get, Double.class);
          • stats.addValue(value);
            + Object n = args.get;
            + if( n != null) {
            + if(n instanceof Iterable) {
            + for(Object num : (Iterable<Object>)n) {
            + if(num != null) {
              • End diff –

          Just a nit, but why 'double' and not Double?
          Is there a reason to box?

          Show
          githubbot ASF GitHub Bot added a comment - Github user ottobackwards commented on a diff in the pull request: https://github.com/apache/metron/pull/750#discussion_r138506017 — Diff: metron-analytics/metron-statistics/src/main/java/org/apache/metron/statistics/StellarStatisticsFunctions.java — @@ -141,8 +141,21 @@ public Object apply(List<Object> args) { // add each of the numeric values for(int i=1; i<args.size(); i++) { double value = convert(args.get , Double.class); stats.addValue(value); + Object n = args.get ; + if( n != null) { + if(n instanceof Iterable) { + for(Object num : (Iterable<Object>)n) { + if(num != null) { End diff – Just a nit, but why 'double' and not Double? Is there a reason to box?
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user ottobackwards commented on a diff in the pull request:

          https://github.com/apache/metron/pull/750#discussion_r138506239

          — Diff: metron-analytics/metron-statistics/README.md —
          @@ -60,7 +60,7 @@ functions can be used from everywhere where Stellar is used.

          • Description: Adds one or more input values to those that are used to calculate the summary statistics.
          • Input:
          • stats - The Stellar statistics object. If null, then a new one is initialized.
          • * value+ - One or more numbers to add
            + * value+ - One or more numbers to add. If one of objects is a list, it will iterate over the list and add each number.
              • End diff –

          "One or more items to add. Each item may be a number or a list of numbers. If an item is a list, each number in the list will be added"?

          Show
          githubbot ASF GitHub Bot added a comment - Github user ottobackwards commented on a diff in the pull request: https://github.com/apache/metron/pull/750#discussion_r138506239 — Diff: metron-analytics/metron-statistics/README.md — @@ -60,7 +60,7 @@ functions can be used from everywhere where Stellar is used. Description: Adds one or more input values to those that are used to calculate the summary statistics. Input: stats - The Stellar statistics object. If null, then a new one is initialized. * value+ - One or more numbers to add + * value+ - One or more numbers to add. If one of objects is a list, it will iterate over the list and add each number. End diff – "One or more items to add. Each item may be a number or a list of numbers. If an item is a list, each number in the list will be added"?
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user ottobackwards commented on the issue:

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

          Looks great. Just a couple of nits, but otherwise LGTM +1.

          Show
          githubbot ASF GitHub Bot added a comment - Github user ottobackwards commented on the issue: https://github.com/apache/metron/pull/750 Looks great. Just a couple of nits, but otherwise LGTM +1.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user cestella commented on a diff in the pull request:

          https://github.com/apache/metron/pull/750#discussion_r138725654

          — Diff: metron-analytics/metron-statistics/src/main/java/org/apache/metron/statistics/StellarStatisticsFunctions.java —
          @@ -141,8 +141,21 @@ public Object apply(List<Object> args) {

          // add each of the numeric values
          for(int i=1; i<args.size(); i++) {

          • double value = convert(args.get, Double.class);
          • stats.addValue(value);
            + Object n = args.get;
            + if( n != null) {
            + if(n instanceof Iterable) {
            + for(Object num : (Iterable<Object>)n) {
            + if(num != null) {
              • End diff –

          No good reason

          Show
          githubbot ASF GitHub Bot added a comment - Github user cestella commented on a diff in the pull request: https://github.com/apache/metron/pull/750#discussion_r138725654 — Diff: metron-analytics/metron-statistics/src/main/java/org/apache/metron/statistics/StellarStatisticsFunctions.java — @@ -141,8 +141,21 @@ public Object apply(List<Object> args) { // add each of the numeric values for(int i=1; i<args.size(); i++) { double value = convert(args.get , Double.class); stats.addValue(value); + Object n = args.get ; + if( n != null) { + if(n instanceof Iterable) { + for(Object num : (Iterable<Object>)n) { + if(num != null) { End diff – No good reason
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user cestella commented on the issue:

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

          Ok, this is updated as per your comments, @ottobackwards

          Show
          githubbot ASF GitHub Bot added a comment - Github user cestella commented on the issue: https://github.com/apache/metron/pull/750 Ok, this is updated as per your comments, @ottobackwards
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user ottobackwards commented on the issue:

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

          +1, thanks for the contribution and great work.

          Show
          githubbot ASF GitHub Bot added a comment - Github user ottobackwards commented on the issue: https://github.com/apache/metron/pull/750 +1, thanks for the contribution and great work.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user asfgit closed the pull request at:

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

          Show
          githubbot ASF GitHub Bot added a comment - Github user asfgit closed the pull request at: https://github.com/apache/metron/pull/750

            People

            • Assignee:
              Unassigned
              Reporter:
              cestella Casey Stella
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:

                Development