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

Posted to reviews@aurora.apache.org by Stephan Erb <se...@apache.org> on 2016/03/22 01:19:28 UTC

Review Request 45135: Descheduling a cron should not fail if the job is not scheduled.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45135/
-----------------------------------------------------------

Review request for Aurora, Maxim Khutornenko and Zameer Manji.


Bugs: AURORA-1645
    https://issues.apache.org/jira/browse/AURORA-1645


Repository: aurora


Description
-------

This allows clients to be more declarative rather than imperative
when expressing that the given job should not be scheduled.


Diffs
-----

  RELEASE-NOTES.md 6e9364e34db6dbb270468db3ff333b956c6bf9f3 
  src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java d217faf44ab3d6132db3b3c4eed67effd03fb6fa 
  src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java c774ac0f0e2fdda7fe9b64fd9181f107b3fd9eca 

Diff: https://reviews.apache.org/r/45135/diff/


Testing
-------

./gradlew -Pq build


Thanks,

Stephan Erb

Re: Review Request 45135: Descheduling a cron should not fail if the job is not scheduled.

Posted by Zameer Manji <zm...@apache.org>.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45135/#review124832
-----------------------------------------------------------


Ship it!




LGTM modulo Maxim's comment.

- Zameer Manji


On March 21, 2016, 5:19 p.m., Stephan Erb wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45135/
> -----------------------------------------------------------
> 
> (Updated March 21, 2016, 5:19 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Zameer Manji.
> 
> 
> Bugs: AURORA-1645
>     https://issues.apache.org/jira/browse/AURORA-1645
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> This allows clients to be more declarative rather than imperative
> when expressing that the given job should not be scheduled.
> 
> 
> Diffs
> -----
> 
>   RELEASE-NOTES.md 6e9364e34db6dbb270468db3ff333b956c6bf9f3 
>   src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java d217faf44ab3d6132db3b3c4eed67effd03fb6fa 
>   src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java c774ac0f0e2fdda7fe9b64fd9181f107b3fd9eca 
> 
> Diff: https://reviews.apache.org/r/45135/diff/
> 
> 
> Testing
> -------
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Stephan Erb
> 
>

Re: Review Request 45135: Descheduling a cron should not fail if the job is not scheduled.

Posted by Aurora ReviewBot <wf...@apache.org>.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45135/#review124693
-----------------------------------------------------------



Master (c5254c7) is red with this patch.
  ./build-support/jenkins/build.sh

                     [1m      proxy_driver = ProxyDriver()[0m
                     [1m      with temporary_dir() as checkpoint_root:[0m
                     [1m        te = AuroraExecutor([0m
                     [1m>           runner_provider=make_provider(checkpoint_root),[0m
                     [1m            sandbox_provider=DefaultTestSandboxProvider())[0m
                     
                     src/test/python/apache/aurora/executor/test_thermos_executor.py:580: 
                     _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
                     src/test/python/apache/aurora/executor/test_thermos_executor.py:193: in make_provider
                     [1m    pex_location=thermos_runner_path(),[0m
                     _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
                     
                     build = True
                     
                     [1m    def thermos_runner_path(build=True):[0m
                     [1m      if not build:[0m
                     [1m        return getattr(thermos_runner_path, 'value', None)[0m
                     [1m    [0m
                     [1m      if not hasattr(thermos_runner_path, 'value'):[0m
                     [1m        pex_dir = safe_mkdtemp()[0m
                     [1m>       assert subprocess.call(["./pants", "--pants-distdir=%s" % pex_dir, "binary",[0m
                     [1m          "src/main/python/apache/thermos/runner:thermos_runner"]) == 0[0m
                     [1m[31mE       assert 1 == 0[0m
                     [1m[31mE        +  where 1 = <function call at 0x7fcb6f7f5938>(['./pants', '--pants-distdir=/tmp/user/2395/tmpypgWqm', 'binary', 'src/main/python/apache/thermos/runner:thermos_runner'])[0m
                     [1m[31mE        +    where <function call at 0x7fcb6f7f5938> = subprocess.call[0m
                     
                     src/test/python/apache/aurora/executor/test_thermos_executor.py:185: AssertionError
                     -------------- Captured stderr call --------------
                     Traceback (most recent call last):
                       File "/home/jenkins/.cache/pants/setup/bootstrap-Linux-x86_64/0.0.75/bin/pants", line 7, in <module>
                         from pants.bin.pants_exe import main
                     ImportError: No module named pants.bin.pants_exe
                      generated xml file: /home/jenkins/jenkins-slave/workspace/AuroraBot/dist/test-results/415337499eb72578eab327a6487c1f5c9452b3d6.xml 
                     [1m[31m 16 failed, 640 passed, 5 skipped, 1 warnings, 8 error in 223.31 seconds [0m
                     
FAILURE


00:34:43 04:33   [complete][31m
               FAILURE[0m


I will refresh this build result if you post a review containing "@ReviewBot retry"

- Aurora ReviewBot


On March 22, 2016, 12:19 a.m., Stephan Erb wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45135/
> -----------------------------------------------------------
> 
> (Updated March 22, 2016, 12:19 a.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Zameer Manji.
> 
> 
> Bugs: AURORA-1645
>     https://issues.apache.org/jira/browse/AURORA-1645
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> This allows clients to be more declarative rather than imperative
> when expressing that the given job should not be scheduled.
> 
> 
> Diffs
> -----
> 
>   RELEASE-NOTES.md 6e9364e34db6dbb270468db3ff333b956c6bf9f3 
>   src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java d217faf44ab3d6132db3b3c4eed67effd03fb6fa 
>   src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java c774ac0f0e2fdda7fe9b64fd9181f107b3fd9eca 
> 
> Diff: https://reviews.apache.org/r/45135/diff/
> 
> 
> Testing
> -------
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Stephan Erb
> 
>

Re: Review Request 45135: Descheduling a cron should not fail if the job is not scheduled.

Posted by Maxim Khutornenko <ma...@apache.org>.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45135/#review124902
-----------------------------------------------------------


Ship it!




Ship It!

- Maxim Khutornenko


On March 22, 2016, 10:21 p.m., Stephan Erb wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45135/
> -----------------------------------------------------------
> 
> (Updated March 22, 2016, 10:21 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Zameer Manji.
> 
> 
> Bugs: AURORA-1645
>     https://issues.apache.org/jira/browse/AURORA-1645
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> This allows clients to be more declarative rather than imperative
> when expressing that the given job should not be scheduled.
> 
> 
> Diffs
> -----
> 
>   RELEASE-NOTES.md 6e9364e34db6dbb270468db3ff333b956c6bf9f3 
>   src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java d217faf44ab3d6132db3b3c4eed67effd03fb6fa 
>   src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java c774ac0f0e2fdda7fe9b64fd9181f107b3fd9eca 
> 
> Diff: https://reviews.apache.org/r/45135/diff/
> 
> 
> Testing
> -------
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Stephan Erb
> 
>

Re: Review Request 45135: Descheduling a cron should not fail if the job is not scheduled.

Posted by Aurora ReviewBot <wf...@apache.org>.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45135/#review124921
-----------------------------------------------------------


Ship it!




Master (b5c9e1b) is green with this patch.
  ./build-support/jenkins/build.sh

I will refresh this build result if you post a review containing "@ReviewBot retry"

- Aurora ReviewBot


On March 22, 2016, 10:21 p.m., Stephan Erb wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45135/
> -----------------------------------------------------------
> 
> (Updated March 22, 2016, 10:21 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Zameer Manji.
> 
> 
> Bugs: AURORA-1645
>     https://issues.apache.org/jira/browse/AURORA-1645
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> This allows clients to be more declarative rather than imperative
> when expressing that the given job should not be scheduled.
> 
> 
> Diffs
> -----
> 
>   RELEASE-NOTES.md 6e9364e34db6dbb270468db3ff333b956c6bf9f3 
>   src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java d217faf44ab3d6132db3b3c4eed67effd03fb6fa 
>   src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java c774ac0f0e2fdda7fe9b64fd9181f107b3fd9eca 
> 
> Diff: https://reviews.apache.org/r/45135/diff/
> 
> 
> Testing
> -------
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Stephan Erb
> 
>

Re: Review Request 45135: Descheduling a cron should not fail if the job is not scheduled.

Posted by Stephan Erb <se...@apache.org>.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45135/
-----------------------------------------------------------

(Updated March 22, 2016, 11:21 p.m.)


Review request for Aurora, Maxim Khutornenko and Zameer Manji.


Changes
-------

Maxim's comment. Message remains at "Job %s is not scheduled with cron"


Bugs: AURORA-1645
    https://issues.apache.org/jira/browse/AURORA-1645


Repository: aurora


Description
-------

This allows clients to be more declarative rather than imperative
when expressing that the given job should not be scheduled.


Diffs (updated)
-----

  RELEASE-NOTES.md 6e9364e34db6dbb270468db3ff333b956c6bf9f3 
  src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java d217faf44ab3d6132db3b3c4eed67effd03fb6fa 
  src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java c774ac0f0e2fdda7fe9b64fd9181f107b3fd9eca 

Diff: https://reviews.apache.org/r/45135/diff/


Testing
-------

./gradlew -Pq build


Thanks,

Stephan Erb

Re: Review Request 45135: Descheduling a cron should not fail if the job is not scheduled.

Posted by Zameer Manji <zm...@apache.org>.


> On March 21, 2016, 8:48 p.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java, line 345
> > <https://reviews.apache.org/r/45135/diff/1/?file=1309594#file1309594line345>
> >
> >     I think we should be consistent and return a message like "No active cron schedule found" in this case similar the `killTasks` behavior.

+1


- Zameer


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45135/#review124726
-----------------------------------------------------------


On March 21, 2016, 5:19 p.m., Stephan Erb wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45135/
> -----------------------------------------------------------
> 
> (Updated March 21, 2016, 5:19 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Zameer Manji.
> 
> 
> Bugs: AURORA-1645
>     https://issues.apache.org/jira/browse/AURORA-1645
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> This allows clients to be more declarative rather than imperative
> when expressing that the given job should not be scheduled.
> 
> 
> Diffs
> -----
> 
>   RELEASE-NOTES.md 6e9364e34db6dbb270468db3ff333b956c6bf9f3 
>   src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java d217faf44ab3d6132db3b3c4eed67effd03fb6fa 
>   src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java c774ac0f0e2fdda7fe9b64fd9181f107b3fd9eca 
> 
> Diff: https://reviews.apache.org/r/45135/diff/
> 
> 
> Testing
> -------
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Stephan Erb
> 
>

Re: Review Request 45135: Descheduling a cron should not fail if the job is not scheduled.

Posted by Maxim Khutornenko <ma...@apache.org>.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45135/#review124726
-----------------------------------------------------------




src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java (line 345)
<https://reviews.apache.org/r/45135/#comment187372>

    I think we should be consistent and return a message like "No active cron schedule found" in this case similar the `killTasks` behavior.


- Maxim Khutornenko


On March 22, 2016, 12:19 a.m., Stephan Erb wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45135/
> -----------------------------------------------------------
> 
> (Updated March 22, 2016, 12:19 a.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Zameer Manji.
> 
> 
> Bugs: AURORA-1645
>     https://issues.apache.org/jira/browse/AURORA-1645
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> This allows clients to be more declarative rather than imperative
> when expressing that the given job should not be scheduled.
> 
> 
> Diffs
> -----
> 
>   RELEASE-NOTES.md 6e9364e34db6dbb270468db3ff333b956c6bf9f3 
>   src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java d217faf44ab3d6132db3b3c4eed67effd03fb6fa 
>   src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java c774ac0f0e2fdda7fe9b64fd9181f107b3fd9eca 
> 
> Diff: https://reviews.apache.org/r/45135/diff/
> 
> 
> Testing
> -------
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Stephan Erb
> 
>