You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@aurora.apache.org by Joe Smith <ya...@gmail.com> on 2015/07/12 20:49:26 UTC
Review Request 36436: Prevent job updates from allowing unbounded
instance events
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36436/
-----------------------------------------------------------
Review request for Aurora and Bill Farner.
Bugs: AURORA-1096
https://issues.apache.org/jira/browse/AURORA-1096
Repository: aurora
Description
-------
Prevent job updates from allowing unbounded instance events
Diffs
-----
src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java dc0cd2d71d11b8157154f7b63a14f0282dee09f1
src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java d28baba7618ebb194a61455971786aef46abd8eb
Diff: https://reviews.apache.org/r/36436/diff/
Testing
-------
`./gradlew build -Pq`
Thanks,
Joe Smith
Re: Review Request 36436: Prevent job updates from allowing unbounded
instance events
Posted by Joe Smith <ya...@gmail.com>.
> On July 14, 2015, 10:55 a.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java, lines 168-177
> > <https://reviews.apache.org/r/36436/diff/2/?file=1010206#file1010206line168>
> >
> > Have you considered a single arg for the upper bound of instance events that are allowed for a job update? I think this would be easier for the operator to reason about, and simpler to associate directly to DB impact.
Good idea, and much simpler. Let me know what you think of the implementation.
Thanks!
- Joe
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36436/#review91645
-----------------------------------------------------------
On July 13, 2015, 4:10 p.m., Joe Smith wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36436/
> -----------------------------------------------------------
>
> (Updated July 13, 2015, 4:10 p.m.)
>
>
> Review request for Aurora and Bill Farner.
>
>
> Bugs: AURORA-1096
> https://issues.apache.org/jira/browse/AURORA-1096
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Prevent job updates from allowing unbounded instance events
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java dc0cd2d71d11b8157154f7b63a14f0282dee09f1
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java d28baba7618ebb194a61455971786aef46abd8eb
>
> Diff: https://reviews.apache.org/r/36436/diff/
>
>
> Testing
> -------
>
> `./gradlew build -Pq`
>
>
> Thanks,
>
> Joe Smith
>
>
Re: Review Request 36436: Prevent job updates from allowing unbounded
instance events
Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36436/#review91645
-----------------------------------------------------------
src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java (lines 168 - 177)
<https://reviews.apache.org/r/36436/#comment145170>
Have you considered a single arg for the upper bound of instance events that are allowed for a job update? I think this would be easier for the operator to reason about, and simpler to associate directly to DB impact.
- Bill Farner
On July 13, 2015, 11:10 p.m., Joe Smith wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36436/
> -----------------------------------------------------------
>
> (Updated July 13, 2015, 11:10 p.m.)
>
>
> Review request for Aurora and Bill Farner.
>
>
> Bugs: AURORA-1096
> https://issues.apache.org/jira/browse/AURORA-1096
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Prevent job updates from allowing unbounded instance events
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java dc0cd2d71d11b8157154f7b63a14f0282dee09f1
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java d28baba7618ebb194a61455971786aef46abd8eb
>
> Diff: https://reviews.apache.org/r/36436/diff/
>
>
> Testing
> -------
>
> `./gradlew build -Pq`
>
>
> Thanks,
>
> Joe Smith
>
>
Re: Review Request 36436: Prevent job updates from allowing unbounded
instance events
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36436/#review91701
-----------------------------------------------------------
Ship it!
Master (d9dac92) 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 July 15, 2015, 12:30 a.m., Joe Smith wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36436/
> -----------------------------------------------------------
>
> (Updated July 15, 2015, 12:30 a.m.)
>
>
> Review request for Aurora and Bill Farner.
>
>
> Bugs: AURORA-1096
> https://issues.apache.org/jira/browse/AURORA-1096
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Prevent job updates from allowing unbounded instance events
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java dc0cd2d71d11b8157154f7b63a14f0282dee09f1
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java d28baba7618ebb194a61455971786aef46abd8eb
>
> Diff: https://reviews.apache.org/r/36436/diff/
>
>
> Testing
> -------
>
> `./gradlew build -Pq`
>
>
> Thanks,
>
> Joe Smith
>
>
Re: Review Request 36436: Prevent job updates from allowing unbounded
instance events
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36436/#review91786
-----------------------------------------------------------
Ship it!
Master (d9dac92) 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 July 15, 2015, 6:08 p.m., Joe Smith wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36436/
> -----------------------------------------------------------
>
> (Updated July 15, 2015, 6:08 p.m.)
>
>
> Review request for Aurora and Bill Farner.
>
>
> Bugs: AURORA-1096
> https://issues.apache.org/jira/browse/AURORA-1096
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Prevent job updates from allowing unbounded instance events
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java dc0cd2d71d11b8157154f7b63a14f0282dee09f1
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java d28baba7618ebb194a61455971786aef46abd8eb
>
> Diff: https://reviews.apache.org/r/36436/diff/
>
>
> Testing
> -------
>
> `./gradlew build -Pq`
>
>
> Thanks,
>
> Joe Smith
>
>
Re: Review Request 36436: Prevent job updates from allowing unbounded
instance events
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36436/#review91815
-----------------------------------------------------------
Ship it!
Master (545e839) 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 July 15, 2015, 8:23 p.m., Joe Smith wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36436/
> -----------------------------------------------------------
>
> (Updated July 15, 2015, 8:23 p.m.)
>
>
> Review request for Aurora and Bill Farner.
>
>
> Bugs: AURORA-1096
> https://issues.apache.org/jira/browse/AURORA-1096
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Prevent job updates from allowing unbounded instance events
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java dc0cd2d71d11b8157154f7b63a14f0282dee09f1
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java d28baba7618ebb194a61455971786aef46abd8eb
>
> Diff: https://reviews.apache.org/r/36436/diff/
>
>
> Testing
> -------
>
> `./gradlew build -Pq`
>
>
> Thanks,
>
> Joe Smith
>
>
Re: Review Request 36436: Prevent job updates from allowing unbounded
instance events
Posted by Joe Smith <ya...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36436/
-----------------------------------------------------------
(Updated July 15, 2015, 1:23 p.m.)
Review request for Aurora and Bill Farner.
Bugs: AURORA-1096
https://issues.apache.org/jira/browse/AURORA-1096
Repository: aurora
Description
-------
Prevent job updates from allowing unbounded instance events
Diffs (updated)
-----
src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java dc0cd2d71d11b8157154f7b63a14f0282dee09f1
src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java d28baba7618ebb194a61455971786aef46abd8eb
Diff: https://reviews.apache.org/r/36436/diff/
Testing
-------
`./gradlew build -Pq`
Thanks,
Joe Smith
Re: Review Request 36436: Prevent job updates from allowing unbounded
instance events
Posted by Joe Smith <ya...@gmail.com>.
> On July 15, 2015, 11:47 a.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java, lines 168-169
> > <https://reviews.apache.org/r/36436/diff/4/?file=1012829#file1012829line168>
> >
> > Sorry, i made that suggestion before i added the comment to break out constants and multiply. Feel free to nuke this :-/
No worries at all! I found this comment to still be ~helpful, but I can see how the constants make this redundant and thus removable.
Thanks again! :)
- Joe
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36436/#review91790
-----------------------------------------------------------
On July 15, 2015, 11:08 a.m., Joe Smith wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36436/
> -----------------------------------------------------------
>
> (Updated July 15, 2015, 11:08 a.m.)
>
>
> Review request for Aurora and Bill Farner.
>
>
> Bugs: AURORA-1096
> https://issues.apache.org/jira/browse/AURORA-1096
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Prevent job updates from allowing unbounded instance events
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java dc0cd2d71d11b8157154f7b63a14f0282dee09f1
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java d28baba7618ebb194a61455971786aef46abd8eb
>
> Diff: https://reviews.apache.org/r/36436/diff/
>
>
> Testing
> -------
>
> `./gradlew build -Pq`
>
>
> Thanks,
>
> Joe Smith
>
>
Re: Review Request 36436: Prevent job updates from allowing unbounded
instance events
Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36436/#review91790
-----------------------------------------------------------
Ship it!
LGTM, i caused one nit - remove that and i'll commit :-)
src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java (lines 168 - 169)
<https://reviews.apache.org/r/36436/#comment145438>
Sorry, i made that suggestion before i added the comment to break out constants and multiply. Feel free to nuke this :-/
- Bill Farner
On July 15, 2015, 6:08 p.m., Joe Smith wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36436/
> -----------------------------------------------------------
>
> (Updated July 15, 2015, 6:08 p.m.)
>
>
> Review request for Aurora and Bill Farner.
>
>
> Bugs: AURORA-1096
> https://issues.apache.org/jira/browse/AURORA-1096
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Prevent job updates from allowing unbounded instance events
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java dc0cd2d71d11b8157154f7b63a14f0282dee09f1
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java d28baba7618ebb194a61455971786aef46abd8eb
>
> Diff: https://reviews.apache.org/r/36436/diff/
>
>
> Testing
> -------
>
> `./gradlew build -Pq`
>
>
> Thanks,
>
> Joe Smith
>
>
Re: Review Request 36436: Prevent job updates from allowing unbounded
instance events
Posted by Joe Smith <ya...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36436/
-----------------------------------------------------------
(Updated July 15, 2015, 11:08 a.m.)
Review request for Aurora and Bill Farner.
Changes
-------
Bill's fixes
Bugs: AURORA-1096
https://issues.apache.org/jira/browse/AURORA-1096
Repository: aurora
Description
-------
Prevent job updates from allowing unbounded instance events
Diffs (updated)
-----
src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java dc0cd2d71d11b8157154f7b63a14f0282dee09f1
src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java d28baba7618ebb194a61455971786aef46abd8eb
Diff: https://reviews.apache.org/r/36436/diff/
Testing
-------
`./gradlew build -Pq`
Thanks,
Joe Smith
Re: Review Request 36436: Prevent job updates from allowing unbounded
instance events
Posted by Joe Smith <ya...@gmail.com>.
> On July 15, 2015, 9:50 a.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java, line 169
> > <https://reviews.apache.org/r/36436/diff/3/?file=1011900#file1011900line169>
> >
> > For better readability, how about `max_update_instance_failures`?
Done.
> On July 15, 2015, 9:50 a.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java, line 171
> > <https://reviews.apache.org/r/36436/diff/3/?file=1011900#file1011900line171>
> >
> > s/scheduler datastore/storage/
Done.
> On July 15, 2015, 9:50 a.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java, line 173
> > <https://reviews.apache.org/r/36436/diff/3/?file=1011900#file1011900line173>
> >
> > Unfortunately, this will not work as you expect and derive from the value of `-max_tasks_per_job`. This will always capture the default value (4000) since it is evaluated before the arg is populated. In fact, this would cause a failure if `-max_tasks_per_job` was set on the command line, since args prevent setting a value after it has been read.
> >
> > The best way i can see to accomplish this while maintaining the relationship is to extract constants:
> > ```
> > private static final int DEFAULT_MAX_TASKS_PER_JOB = 4000;
> > private static final int DEFAULT_MAX_INSTANCE_UPDATE_EVENT_FAILURES =
> > DEFAULT_MAX_TASKS_PER_JOB * 5;
> > ```
> >
> > There isn't a clean way to have this multiplier effect that Just Works, so i suggest you just set it to 20000 and leave a comment near `MAX_TASKS_PER_JOB` suggesting that this arg be changed if the default is changed there.
Aha, good to know. Thanks for catching that! Let me know if you think that comment is sufficient.
> On July 15, 2015, 9:50 a.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java, line 1123
> > <https://reviews.apache.org/r/36436/diff/3/?file=1011900#file1011900line1123>
> >
> > This first part can be ommitted, since it's effectively covered by `MAX_TASKS_PER_JOB`.
Done.
- Joe
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36436/#review91749
-----------------------------------------------------------
On July 14, 2015, 5:30 p.m., Joe Smith wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36436/
> -----------------------------------------------------------
>
> (Updated July 14, 2015, 5:30 p.m.)
>
>
> Review request for Aurora and Bill Farner.
>
>
> Bugs: AURORA-1096
> https://issues.apache.org/jira/browse/AURORA-1096
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Prevent job updates from allowing unbounded instance events
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java dc0cd2d71d11b8157154f7b63a14f0282dee09f1
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java d28baba7618ebb194a61455971786aef46abd8eb
>
> Diff: https://reviews.apache.org/r/36436/diff/
>
>
> Testing
> -------
>
> `./gradlew build -Pq`
>
>
> Thanks,
>
> Joe Smith
>
>
Re: Review Request 36436: Prevent job updates from allowing unbounded
instance events
Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36436/#review91749
-----------------------------------------------------------
src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java (line 169)
<https://reviews.apache.org/r/36436/#comment145378>
For better readability, how about `max_update_instance_failures`?
src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java (line 171)
<https://reviews.apache.org/r/36436/#comment145377>
s/scheduler datastore/storage/
src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java (line 173)
<https://reviews.apache.org/r/36436/#comment145375>
Unfortunately, this will not work as you expect and derive from the value of `-max_tasks_per_job`. This will always capture the default value (4000) since it is evaluated before the arg is populated. In fact, this would cause a failure if `-max_tasks_per_job` was set on the command line, since args prevent setting a value after it has been read.
The best way i can see to accomplish this while maintaining the relationship is to extract constants:
```
private static final int DEFAULT_MAX_TASKS_PER_JOB = 4000;
private static final int DEFAULT_MAX_INSTANCE_UPDATE_EVENT_FAILURES =
DEFAULT_MAX_TASKS_PER_JOB * 5;
```
There isn't a clean way to have this multiplier effect that Just Works, so i suggest you just set it to 20000 and leave a comment near `MAX_TASKS_PER_JOB` suggesting that this arg be changed if the default is changed there.
src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java (line 1123)
<https://reviews.apache.org/r/36436/#comment145376>
This first part can be ommitted, since it's effectively covered by `MAX_TASKS_PER_JOB`.
- Bill Farner
On July 15, 2015, 12:30 a.m., Joe Smith wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36436/
> -----------------------------------------------------------
>
> (Updated July 15, 2015, 12:30 a.m.)
>
>
> Review request for Aurora and Bill Farner.
>
>
> Bugs: AURORA-1096
> https://issues.apache.org/jira/browse/AURORA-1096
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Prevent job updates from allowing unbounded instance events
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java dc0cd2d71d11b8157154f7b63a14f0282dee09f1
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java d28baba7618ebb194a61455971786aef46abd8eb
>
> Diff: https://reviews.apache.org/r/36436/diff/
>
>
> Testing
> -------
>
> `./gradlew build -Pq`
>
>
> Thanks,
>
> Joe Smith
>
>
Re: Review Request 36436: Prevent job updates from allowing unbounded
instance events
Posted by Joe Smith <ya...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36436/
-----------------------------------------------------------
(Updated July 14, 2015, 5:30 p.m.)
Review request for Aurora and Bill Farner.
Bugs: AURORA-1096
https://issues.apache.org/jira/browse/AURORA-1096
Repository: aurora
Description
-------
Prevent job updates from allowing unbounded instance events
Diffs (updated)
-----
src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java dc0cd2d71d11b8157154f7b63a14f0282dee09f1
src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java d28baba7618ebb194a61455971786aef46abd8eb
Diff: https://reviews.apache.org/r/36436/diff/
Testing
-------
`./gradlew build -Pq`
Thanks,
Joe Smith
Re: Review Request 36436: Prevent job updates from allowing unbounded
instance events
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36436/#review91561
-----------------------------------------------------------
Ship it!
Master (b7a02a5) 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 July 13, 2015, 11:10 p.m., Joe Smith wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36436/
> -----------------------------------------------------------
>
> (Updated July 13, 2015, 11:10 p.m.)
>
>
> Review request for Aurora and Bill Farner.
>
>
> Bugs: AURORA-1096
> https://issues.apache.org/jira/browse/AURORA-1096
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Prevent job updates from allowing unbounded instance events
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java dc0cd2d71d11b8157154f7b63a14f0282dee09f1
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java d28baba7618ebb194a61455971786aef46abd8eb
>
> Diff: https://reviews.apache.org/r/36436/diff/
>
>
> Testing
> -------
>
> `./gradlew build -Pq`
>
>
> Thanks,
>
> Joe Smith
>
>
Re: Review Request 36436: Prevent job updates from allowing unbounded
instance events
Posted by Joe Smith <ya...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36436/
-----------------------------------------------------------
(Updated July 13, 2015, 4:10 p.m.)
Review request for Aurora and Bill Farner.
Bugs: AURORA-1096
https://issues.apache.org/jira/browse/AURORA-1096
Repository: aurora
Description
-------
Prevent job updates from allowing unbounded instance events
Diffs (updated)
-----
src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java dc0cd2d71d11b8157154f7b63a14f0282dee09f1
src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java d28baba7618ebb194a61455971786aef46abd8eb
Diff: https://reviews.apache.org/r/36436/diff/
Testing
-------
`./gradlew build -Pq`
Thanks,
Joe Smith
Re: Review Request 36436: Prevent job updates from allowing unbounded
instance events
Posted by Joe Smith <ya...@gmail.com>.
> On July 12, 2015, 11:38 p.m., Stephan Erb wrote:
> > src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java, line 169
> > <https://reviews.apache.org/r/36436/diff/1/?file=1009204#file1009204line169>
> >
> > Give that this will be the only documentation of this feature, the doc string could be a bit longer and explain that this option applies to the job update feature and limits the `max_per_shard_failures` jobspec configuration.
Done (I believe, thatis. Let me know if you think this is clearer!)
> On July 12, 2015, 11:38 p.m., Stephan Erb wrote:
> > src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java, line 173
> > <https://reviews.apache.org/r/36436/diff/1/?file=1009204#file1009204line173>
> >
> > Same as above. It is hard to understand the implications of this option without reading the code.
Done. (But let me know what you think!)
- Joe
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36436/#review91416
-----------------------------------------------------------
On July 12, 2015, 11:49 a.m., Joe Smith wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36436/
> -----------------------------------------------------------
>
> (Updated July 12, 2015, 11:49 a.m.)
>
>
> Review request for Aurora and Bill Farner.
>
>
> Bugs: AURORA-1096
> https://issues.apache.org/jira/browse/AURORA-1096
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Prevent job updates from allowing unbounded instance events
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java dc0cd2d71d11b8157154f7b63a14f0282dee09f1
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java d28baba7618ebb194a61455971786aef46abd8eb
>
> Diff: https://reviews.apache.org/r/36436/diff/
>
>
> Testing
> -------
>
> `./gradlew build -Pq`
>
>
> Thanks,
>
> Joe Smith
>
>
Re: Review Request 36436: Prevent job updates from allowing unbounded
instance events
Posted by Stephan Erb <st...@dev.static-void.de>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36436/#review91416
-----------------------------------------------------------
src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java (line 169)
<https://reviews.apache.org/r/36436/#comment144781>
Give that this will be the only documentation of this feature, the doc string could be a bit longer and explain that this option applies to the job update feature and limits the `max_per_shard_failures` jobspec configuration.
src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java (line 173)
<https://reviews.apache.org/r/36436/#comment144795>
Same as above. It is hard to understand the implications of this option without reading the code.
- Stephan Erb
On July 12, 2015, 8:49 p.m., Joe Smith wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36436/
> -----------------------------------------------------------
>
> (Updated July 12, 2015, 8:49 p.m.)
>
>
> Review request for Aurora and Bill Farner.
>
>
> Bugs: AURORA-1096
> https://issues.apache.org/jira/browse/AURORA-1096
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Prevent job updates from allowing unbounded instance events
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java dc0cd2d71d11b8157154f7b63a14f0282dee09f1
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java d28baba7618ebb194a61455971786aef46abd8eb
>
> Diff: https://reviews.apache.org/r/36436/diff/
>
>
> Testing
> -------
>
> `./gradlew build -Pq`
>
>
> Thanks,
>
> Joe Smith
>
>
Re: Review Request 36436: Prevent job updates from allowing unbounded
instance events
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36436/#review91413
-----------------------------------------------------------
Ship it!
Master (190daed) 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 July 12, 2015, 6:49 p.m., Joe Smith wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36436/
> -----------------------------------------------------------
>
> (Updated July 12, 2015, 6:49 p.m.)
>
>
> Review request for Aurora and Bill Farner.
>
>
> Bugs: AURORA-1096
> https://issues.apache.org/jira/browse/AURORA-1096
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Prevent job updates from allowing unbounded instance events
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java dc0cd2d71d11b8157154f7b63a14f0282dee09f1
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java d28baba7618ebb194a61455971786aef46abd8eb
>
> Diff: https://reviews.apache.org/r/36436/diff/
>
>
> Testing
> -------
>
> `./gradlew build -Pq`
>
>
> Thanks,
>
> Joe Smith
>
>