You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zeppelin.apache.org by RPCMoritz <gi...@git.apache.org> on 2015/09/24 13:28:11 UTC

[GitHub] incubator-zeppelin pull request: [ZEPPELIN-319]wait an additional ...

GitHub user RPCMoritz opened a pull request:

    https://github.com/apache/incubator-zeppelin/pull/323

    [ZEPPELIN-319]wait an additional .5 seconds, if assertion will fail

    This test failed randomly during a build. To make sure this is not due to timing issues, we can optionally wait an additional .5 seconds.
    This is a band-aid around bad design, but I'm pragmatically trying to get the build to be more stable. This should also help determine if there is an actual (randomly striking) issue in the underlying procedure being tested, by making the test more reliable.
    
    Please review, and if you can come up with a better designed test (and avoid waiting for threads to finish in a fixed time interval) we should discuss it here or in the issue.

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

    $ git pull https://github.com/RPCMoritz/incubator-zeppelin ZEPPELIN-319

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

    https://github.com/apache/incubator-zeppelin/pull/323.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 #323
    
----
commit 528dbfaa93fc4e91bf20f30e1498907271073e1a
Author: Rick Moritz <ra...@gmail.com>
Date:   2015-09-24T09:54:27Z

    ZEPPELIN-319 optionally wait an additional .5 seconds, if assertion can be expected to fail
    
    This test failed randomly during a build. To make sure this is not due to timing issues, we can optionally wait an additional .5 seconds.
    This is a band-aid around bad design, but I'm pragmatically trying to get the build to be more stable.
    
    Please review, and if you can come up with a better designed test we should discuss it here or in the issue.

----


---
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] incubator-zeppelin pull request: [ZEPPELIN-319]wait an additional ...

Posted by Leemoonsoo <gi...@git.apache.org>.
Github user Leemoonsoo commented on the pull request:

    https://github.com/apache/incubator-zeppelin/pull/323#issuecomment-145322677
  
    Then, you can break the loop, like
    ```
        // wait for job finish, maximum 10 seconds
        for (int i = 0 ; i < 100 && !job.isTerminated(); i++) {
          Thread.sleep(100);
        }
    ```



---
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] incubator-zeppelin pull request: [ZEPPELIN-319]wait an additional ...

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

    https://github.com/apache/incubator-zeppelin/pull/323


---
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] incubator-zeppelin pull request: [ZEPPELIN-319]wait an additional ...

Posted by RPCMoritz <gi...@git.apache.org>.
GitHub user RPCMoritz reopened a pull request:

    https://github.com/apache/incubator-zeppelin/pull/323

    [ZEPPELIN-319]wait an additional .5 seconds, if assertion will fail

    This test failed randomly during a build. To make sure this is not due to timing issues, we can optionally wait an additional .5 seconds.
    This is a band-aid around bad design, but I'm pragmatically trying to get the build to be more stable. This should also help determine if there is an actual (randomly striking) issue in the underlying procedure being tested, by making the test more reliable.
    
    Please review, and if you can come up with a better designed test (and avoid waiting for threads to finish in a fixed time interval) we should discuss it here or in the issue.

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

    $ git pull https://github.com/RPCMoritz/incubator-zeppelin ZEPPELIN-319

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

    https://github.com/apache/incubator-zeppelin/pull/323.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 #323
    
----
commit 528dbfaa93fc4e91bf20f30e1498907271073e1a
Author: Rick Moritz <ra...@gmail.com>
Date:   2015-09-24T09:54:27Z

    ZEPPELIN-319 optionally wait an additional .5 seconds, if assertion can be expected to fail
    
    This test failed randomly during a build. To make sure this is not due to timing issues, we can optionally wait an additional .5 seconds.
    This is a band-aid around bad design, but I'm pragmatically trying to get the build to be more stable.
    
    Please review, and if you can come up with a better designed test we should discuss it here or in the issue.

commit 0455a66f8a7ed35df6780f1a7bd064d2669f060c
Author: Rick Moritz <ra...@gmail.com>
Date:   2015-10-05T08:57:33Z

    Refactoring and moving to job.isRunning check
    
    Refactored out some key timing variables, adopted same checking procedure for job.isRunning in both before and after checks, added dynamic timing with 10s (100x100ms) max tolerance.

commit 10fd22d1d9d45ceca598dd4ce25df55b0c8fed7a
Author: Rick Moritz <ra...@gmail.com>
Date:   2015-10-05T09:29:34Z

    Fix build and cosmetics
    
    Hopefully this will fix it.

commit 5e4b8d3695b0b141efbcb88f0916118a1f4d993a
Author: Rick Moritz <ra...@gmail.com>
Date:   2015-10-05T09:34:44Z

    added assertEquals import

commit 27b660875d048e53630a41b41cb97af7bc19aefb
Author: Rick Moritz <ra...@gmail.com>
Date:   2015-10-05T14:59:26Z

    Switched to isTerminated() for clearer semantics
    
    isRunning and isTerminated /should/ be disjoint and the only two relevant states in this case, but from a semantics point of view, using isTerminated in the second part makes more sense.

----


---
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] incubator-zeppelin pull request: [ZEPPELIN-319]wait an additional ...

Posted by RPCMoritz <gi...@git.apache.org>.
Github user RPCMoritz commented on the pull request:

    https://github.com/apache/incubator-zeppelin/pull/323#issuecomment-145272322
  
    Well, we do need some kind of way out of that loop, to get to the failure - stage. Infinite looping won't 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] incubator-zeppelin pull request: [ZEPPELIN-319]wait an additional ...

Posted by RPCMoritz <gi...@git.apache.org>.
Github user RPCMoritz commented on the pull request:

    https://github.com/apache/incubator-zeppelin/pull/323#issuecomment-145776128
  
    Looks like npm-dependency-resolution broke the build - how can I trigger a re-check? (Ironic, that this happens, while I'm trying to stabilize the build ^^;)


---
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] incubator-zeppelin pull request: [ZEPPELIN-319]wait an additional ...

Posted by RPCMoritz <gi...@git.apache.org>.
Github user RPCMoritz commented on the pull request:

    https://github.com/apache/incubator-zeppelin/pull/323#issuecomment-145323504
  
    That's more reasonable,  I'll amend the 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] incubator-zeppelin pull request: [ZEPPELIN-319]wait an additional ...

Posted by Leemoonsoo <gi...@git.apache.org>.
Github user Leemoonsoo commented on the pull request:

    https://github.com/apache/incubator-zeppelin/pull/323#issuecomment-145229861
  
    Purpose of `Thread.sleep(500)` at line 119 is waiting for the job finish.
    So, how about replace it with 
    ```
        while (!job.isTerminated()) {
          Thread.sleep(100);
        }
    ```
    ?
    



---
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] incubator-zeppelin pull request: [ZEPPELIN-319]wait an additional ...

Posted by Leemoonsoo <gi...@git.apache.org>.
Github user Leemoonsoo commented on the pull request:

    https://github.com/apache/incubator-zeppelin/pull/323#issuecomment-147168031
  
    Thanks! LGTM


---
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] incubator-zeppelin pull request: [ZEPPELIN-319]wait an additional ...

Posted by RPCMoritz <gi...@git.apache.org>.
Github user RPCMoritz commented on the pull request:

    https://github.com/apache/incubator-zeppelin/pull/323#issuecomment-145782077
  
    @corneadoug thanks!


---
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] incubator-zeppelin pull request: [ZEPPELIN-319]wait an additional ...

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

    https://github.com/apache/incubator-zeppelin/pull/323


---
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] incubator-zeppelin pull request: [ZEPPELIN-319]wait an additional ...

Posted by corneadoug <gi...@git.apache.org>.
Github user corneadoug commented on the pull request:

    https://github.com/apache/incubator-zeppelin/pull/323#issuecomment-145777022
  
    @RPCMoritz you can close and reopen this PR to trigger the CI again


---
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] incubator-zeppelin pull request: [ZEPPELIN-319]wait an additional ...

Posted by corneadoug <gi...@git.apache.org>.
Github user corneadoug commented on the pull request:

    https://github.com/apache/incubator-zeppelin/pull/323#issuecomment-144600015
  
    @Leemoonsoo any feedback on this 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.
---