You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@metron.apache.org by mmiklavc <gi...@git.apache.org> on 2018/07/24 03:17:25 UTC

[GitHub] metron pull request #1128: METRON-1690: Add more context to PcapJob JobStatu...

GitHub user mmiklavc opened a pull request:

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

    METRON-1690: Add more context to PcapJob JobStatus

    ## Contributor Comments
    
    https://issues.apache.org/jira/browse/METRON-1690
    
    Adds more context for failure scenarios to the JobStatus object returned by PcapJob. Removes duplication between Timer status updates and the getStatus call.
    
    ## Pull Request Checklist
    
    Follow test plan from the following: https://github.com/apache/metron/pull/1109
    
    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:
    - [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 metron folder via:
      ```
      mvn -q clean integration-test install && dev-utilities/build-utils/verify_licenses.sh 
      ```
    
    - [x] Have you written or updated unit tests and or integration tests to verify your changes?
    - [ ] 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/mmiklavc/metron pcap-job-status

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

    https://github.com/apache/metron/pull/1128.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 #1128
    
----
commit 92b0503eef5b3cc41c4b7b99cc8db173715172e2
Author: Michael Miklavcic <mi...@...>
Date:   2018-07-20T18:20:11Z

    Create copy constructor for JobStatus

commit 414ef1015f0d834a174e09557680f2563b15264b
Author: Michael Miklavcic <mi...@...>
Date:   2018-07-24T02:48:42Z

    Provide more context around error conditions.

commit 497e54b71bac94cace9e7ac5d0af982a45366336
Author: Michael Miklavcic <mi...@...>
Date:   2018-07-24T03:10:29Z

    Update diagrams

----


---

[GitHub] metron issue #1128: METRON-1690: Add more context to PcapJob JobStatus

Posted by cestella <gi...@git.apache.org>.
Github user cestella commented on the issue:

    https://github.com/apache/metron/pull/1128
  
    +1 by inspection


---

[GitHub] metron pull request #1128: METRON-1690: Add more context to PcapJob JobStatu...

Posted by mmiklavc <gi...@git.apache.org>.
Github user mmiklavc closed the pull request at:

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


---

[GitHub] metron issue #1128: METRON-1690: Add more context to PcapJob JobStatus

Posted by mmiklavc <gi...@git.apache.org>.
Github user mmiklavc commented on the issue:

    https://github.com/apache/metron/pull/1128
  
    @merrimanr - heads up this has been updated to address the jobId missing on submission


---

[GitHub] metron issue #1128: METRON-1690: Add more context to PcapJob JobStatus

Posted by merrimanr <gi...@git.apache.org>.
Github user merrimanr commented on the issue:

    https://github.com/apache/metron/pull/1128
  
    Actually I did find a problem.  When I submit a normal query where results are included in the time range, I get:
    ```
    {
      "jobId": "",
      "jobStatus": "SUBMITTED",
      "description": "Job submitted",
      "percentComplete": 0,
      "pageTotal": 0
    }
    ```
    The jobId is missing.


---

[GitHub] metron issue #1128: METRON-1690: Add more context to PcapJob JobStatus

Posted by mmiklavc <gi...@git.apache.org>.
Github user mmiklavc commented on the issue:

    https://github.com/apache/metron/pull/1128
  
    @merrimanr - I added "SUBMITTED" as a new separate status where the MR job has been submitted but we potentially don't have a job ID yet. Let me check the API on whether we can return that reliably or not.


---

[GitHub] metron issue #1128: METRON-1690: Add more context to PcapJob JobStatus

Posted by merrimanr <gi...@git.apache.org>.
Github user merrimanr commented on the issue:

    https://github.com/apache/metron/pull/1128
  
    I tested this again and it looks like the job id is present now.  +1


---

[GitHub] metron issue #1128: METRON-1690: Add more context to PcapJob JobStatus

Posted by justinleet <gi...@git.apache.org>.
Github user justinleet commented on the issue:

    https://github.com/apache/metron/pull/1128
  
    +1, thanks for the contribution!


---

[GitHub] metron issue #1128: METRON-1690: Add more context to PcapJob JobStatus

Posted by mmiklavc <gi...@git.apache.org>.
Github user mmiklavc commented on the issue:

    https://github.com/apache/metron/pull/1128
  
    Merged into feature branch.


---

[GitHub] metron issue #1128: METRON-1690: Add more context to PcapJob JobStatus

Posted by merrimanr <gi...@git.apache.org>.
Github user merrimanr commented on the issue:

    https://github.com/apache/metron/pull/1128
  
    I tested this in full dev by submitting a query with a time range that won't match the pcap data I have in `/apps/metron/pcap/input`:
    ```
    curl -X POST --header 'Content-Type: application/json' --header 'Accept: application/json' -d '{
    "startTimeMs": 1500923776000
    }' 'http://node1:8082/api/v1/pcap/fixed'
    ```
    An appropriate message was returned:
    ```
    {
      "jobId": "",
      "jobStatus": "SUCCEEDED",
      "description": "No results in specified date range.",
      "percentComplete": 100,
      "pageTotal": 0
    }
    ```
    I also ran a query with an input path that doesn't have any data:
    ```
    curl -X POST --header 'Content-Type: application/json' --header 'Accept: application/json' -d '{
    "basePath":"/apps/metron/pcap/input2"
    }' 'http://node1:8082/api/v1/pcap/fixed'
    ```
    and again received an appropriate response:
    ```
    {
      "jobId": "",
      "jobStatus": "SUCCEEDED",
      "description": "No results in specified date range.",
      "percentComplete": 100,
      "pageTotal": 0
    }
    ```
    +1 from me.


---