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/21 19:15:33 UTC
[GitHub] incubator-zeppelin pull request: #315 Fix time-dependant schedulin...
GitHub user RPCMoritz opened a pull request:
https://github.com/apache/incubator-zeppelin/pull/315
#315 Fix time-dependant scheduling test
test-schedule can fail when it starts late in a second, as the result will then return the next second due to in-system delays (particularly cron not reacting in time to config.put(cron, null)).
Adding an additional waiting second before measuring dateFinished allows us to take a value that is independent of a possible lagging execution, waiting another second makes sure that the process has properly stopped.
Alternatively, we can test the time and make sure to not write to cron when just before the full second - that would be quicker (less waiting) but slightly more hacky.
Please review.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/RPCMoritz/incubator-zeppelin patch-1
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-zeppelin/pull/315.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 #315
----
commit 6215563857c21d7ef2878565e9e8b459196b7004
Author: Rick Moritz <ra...@gmail.com>
Date: 2015-09-21T17:10:37Z
Fix time-dependant scheduling test
test-schedule can fail when it starts late in a second, as the result will then return the next second due to in-system delays (particularly cron not reacting in time to config.put(cron, null)).
Adding an additional waiting second before measuring dateFinished allows us to take a value that is independent of a possible lagging execution, waiting another second makes sure that the process has properly stopped.
Alternatively, we can test the time and make sure to not write to cron when just before the full second - that would be quicker (less waiting) but slightly more hacky.
Please review.
----
---
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-315 Fix time-dependant s...
Posted by RPCMoritz <gi...@git.apache.org>.
Github user RPCMoritz commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/315#issuecomment-150604474
Rebased 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] incubator-zeppelin pull request: ZEPPELIN-315 Fix time-dependant s...
Posted by RPCMoritz <gi...@git.apache.org>.
GitHub user RPCMoritz reopened a pull request:
https://github.com/apache/incubator-zeppelin/pull/315
ZEPPELIN-315 Fix time-dependant scheduling test
test-schedule can fail when it starts late in a second, as the result will then return the next second due to in-system delays (particularly cron not reacting in time to config.put(cron, null)).
Adding an additional waiting second before measuring dateFinished allows us to take a value that is independent of a possible lagging execution, waiting another second makes sure that the process has properly stopped.
Alternatively, we can test the time and make sure to not write to cron when just before the full second - that would be quicker (less waiting) but slightly more hacky.
Please review.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/RPCMoritz/incubator-zeppelin patch-1
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-zeppelin/pull/315.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 #315
----
commit 42ed8e1f670bafd6aec2ef970c70a6f6d634f8eb
Author: Rick Moritz <ra...@gmail.com>
Date: 2015-09-21T17:10:37Z
Fix time-dependant scheduling test
test-schedule can fail when it starts late in a second, as the result will then return the next second due to in-system delays (particularly cron not reacting in time to config.put(cron, null)).
Adding an additional waiting second before measuring dateFinished allows us to take a value that is independent of a possible lagging execution, waiting another second makes sure that the process has properly stopped.
Alternatively, we can test the time and make sure to not write to cron when just before the full second - that would be quicker (less waiting) but slightly more hacky.
Please review.
----
---
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-315 Fix time-dependant s...
Posted by RPCMoritz <gi...@git.apache.org>.
Github user RPCMoritz commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/315#issuecomment-150260023
Looks like your last commit broke diff's ability to properly locate these lines. Should I attempt to rebase, or can you take care of this during the merge?
---
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-315 Fix time-dependant s...
Posted by RPCMoritz <gi...@git.apache.org>.
Github user RPCMoritz commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/315#issuecomment-146189825
@Leemoonsoo could you take a look at this one? It's a pretty straighforward fix.
---
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-315 Fix time-dependant s...
Posted by RPCMoritz <gi...@git.apache.org>.
Github user RPCMoritz commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/315#issuecomment-147173875
This merely gives the cron configuration one second to become effective, before testing whether the job is still running, using the same logic as before.
---
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-315 Fix time-dependant s...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-zeppelin/pull/315
---
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-315 Fix time-dependant s...
Posted by RPCMoritz <gi...@git.apache.org>.
Github user RPCMoritz closed the pull request at:
https://github.com/apache/incubator-zeppelin/pull/315
---
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-315 Fix time-dependant s...
Posted by Leemoonsoo <gi...@git.apache.org>.
Github user Leemoonsoo commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/315#issuecomment-147171849
The test case was intended to test,
1) create a paragraph
2) set cron expression and wait for paragraph finishes. to verify cron scheduler work.
3) remove cron expression and wait some time, to verify removing cron scheduler works and paragraph is no longer executed.
I think this PR does not validate 3)
---
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-315 Fix time-dependant s...
Posted by Leemoonsoo <gi...@git.apache.org>.
Github user Leemoonsoo commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/315#issuecomment-150468613
Appreciated If you rebase.
---
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-315 Fix time-dependant s...
Posted by FRosner <gi...@git.apache.org>.
Github user FRosner commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/315#issuecomment-155467136
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-315 Fix time-dependant s...
Posted by Leemoonsoo <gi...@git.apache.org>.
Github user Leemoonsoo commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/315#issuecomment-155444690
Okay, Merging it if there're no more discussions.
---
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-315 Fix time-dependant s...
Posted by RPCMoritz <gi...@git.apache.org>.
Github user RPCMoritz commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/315#issuecomment-155355447
@Leemoonsoo I'd really like to be able to tick this one off of my list - any chance of merging this soon?
---
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-315 Fix time-dependant s...
Posted by RPCMoritz <gi...@git.apache.org>.
Github user RPCMoritz commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/315#issuecomment-151411072
Unclear what caused that build to fail - retrying
---
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.
---