You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@aurora.apache.org by Maxim Khutornenko <ma...@apache.org> on 2014/10/28 02:43:11 UTC

Review Request 27262: Adding missing JobConfiguration backfill for task config job field.

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

Review request for Aurora and Bill Farner.


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


Repository: aurora


Description
-------

Adding missing JobConfiguration backfill for task config job field.


Diffs
-----

  src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java 97517ed609289f584e07620d24f6fc09c54d36fc 
  src/test/java/org/apache/aurora/scheduler/storage/StorageBackfillTest.java 32a90d5a7819404a3d9e0b94168a447398607e56 

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


Testing
-------

./gradlew -Pq build


Thanks,

Maxim Khutornenko


Re: Review Request 27262: Adding missing JobConfiguration backfill for task config job field.

Posted by Bill Farner <wf...@apache.org>.

> On Oct. 28, 2014, 2:01 a.m., Aurora ReviewBot wrote:
> > Master (9fb2ce4) is red with this patch.
> >   ./build-support/jenkins/build.sh
> > 
> > src.test.python.apache.aurora.client.commands.ssh                               .....   SUCCESS
> > src.test.python.apache.aurora.client.config                                     .....   SUCCESS
> > src.test.python.apache.aurora.client.hooks.hooked_api                           .....   SUCCESS
> > src.test.python.apache.aurora.client.hooks.non_hooked_api                       .....   SUCCESS
> > src.test.python.apache.aurora.common.test_aurora_job_key                        .....   SUCCESS
> > src.test.python.apache.aurora.common.test_cluster                               .....   SUCCESS
> > src.test.python.apache.aurora.common.test_cluster_option                        .....   SUCCESS
> > src.test.python.apache.aurora.common.test_clusters                              .....   SUCCESS
> > src.test.python.apache.aurora.common.test_http_signaler                         .....   SUCCESS
> > src.test.python.apache.aurora.common.test_shellify                              .....   SUCCESS
> > src.test.python.apache.aurora.common.test_transport                             .....   SUCCESS
> > src.test.python.apache.aurora.config.test_base                                  .....   SUCCESS
> > src.test.python.apache.aurora.config.test_constraint_parsing                    .....   SUCCESS
> > src.test.python.apache.aurora.config.test_loader                                .....   SUCCESS
> > src.test.python.apache.aurora.config.test_thrift                                .....   SUCCESS
> > src.test.python.apache.aurora.executor.executor_base                            .....   SUCCESS
> > src.test.python.apache.aurora.executor.executor_detector                        .....   SUCCESS
> > src.test.python.apache.aurora.executor.executor_vars                            .....   SUCCESS
> > src.test.python.apache.aurora.executor.status_manager                           .....   SUCCESS
> > src.test.python.apache.aurora.executor.thermos_task_runner                      .....   FAILURE

Presumably this is a flaky test, as the diff only touches java :-/


- Bill


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


On Oct. 28, 2014, 1:43 a.m., Maxim Khutornenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27262/
> -----------------------------------------------------------
> 
> (Updated Oct. 28, 2014, 1:43 a.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Bugs: AURORA-899
>     https://issues.apache.org/jira/browse/AURORA-899
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Adding missing JobConfiguration backfill for task config job field.
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java 97517ed609289f584e07620d24f6fc09c54d36fc 
>   src/test/java/org/apache/aurora/scheduler/storage/StorageBackfillTest.java 32a90d5a7819404a3d9e0b94168a447398607e56 
> 
> Diff: https://reviews.apache.org/r/27262/diff/
> 
> 
> Testing
> -------
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>


Re: Review Request 27262: Adding missing JobConfiguration backfill for task config job field.

Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27262/#review58754
-----------------------------------------------------------


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

src.test.python.apache.aurora.client.commands.ssh                               .....   SUCCESS
src.test.python.apache.aurora.client.config                                     .....   SUCCESS
src.test.python.apache.aurora.client.hooks.hooked_api                           .....   SUCCESS
src.test.python.apache.aurora.client.hooks.non_hooked_api                       .....   SUCCESS
src.test.python.apache.aurora.common.test_aurora_job_key                        .....   SUCCESS
src.test.python.apache.aurora.common.test_cluster                               .....   SUCCESS
src.test.python.apache.aurora.common.test_cluster_option                        .....   SUCCESS
src.test.python.apache.aurora.common.test_clusters                              .....   SUCCESS
src.test.python.apache.aurora.common.test_http_signaler                         .....   SUCCESS
src.test.python.apache.aurora.common.test_shellify                              .....   SUCCESS
src.test.python.apache.aurora.common.test_transport                             .....   SUCCESS
src.test.python.apache.aurora.config.test_base                                  .....   SUCCESS
src.test.python.apache.aurora.config.test_constraint_parsing                    .....   SUCCESS
src.test.python.apache.aurora.config.test_loader                                .....   SUCCESS
src.test.python.apache.aurora.config.test_thrift                                .....   SUCCESS
src.test.python.apache.aurora.executor.executor_base                            .....   SUCCESS
src.test.python.apache.aurora.executor.executor_detector                        .....   SUCCESS
src.test.python.apache.aurora.executor.executor_vars                            .....   SUCCESS
src.test.python.apache.aurora.executor.status_manager                           .....   SUCCESS
src.test.python.apache.aurora.executor.thermos_task_runner                      .....   FAILURE

- Aurora ReviewBot


On Oct. 28, 2014, 1:43 a.m., Maxim Khutornenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27262/
> -----------------------------------------------------------
> 
> (Updated Oct. 28, 2014, 1:43 a.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Bugs: AURORA-899
>     https://issues.apache.org/jira/browse/AURORA-899
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Adding missing JobConfiguration backfill for task config job field.
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java 97517ed609289f584e07620d24f6fc09c54d36fc 
>   src/test/java/org/apache/aurora/scheduler/storage/StorageBackfillTest.java 32a90d5a7819404a3d9e0b94168a447398607e56 
> 
> Diff: https://reviews.apache.org/r/27262/diff/
> 
> 
> Testing
> -------
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>


Re: Review Request 27262: Adding missing JobConfiguration backfill for task config job field.

Posted by Bill Farner <wf...@apache.org>.

> On Oct. 28, 2014, 5:30 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java, line 129
> > <https://reviews.apache.org/r/27262/diff/2/?file=735497#file735497line129>
> >
> >     Can you fully centralize the backfill and the counter increment?
> >     
> >         void populateJobKey(TaskConfig config) {
> >           if job key is valid:
> >             fill job key
> >             increment counter
> >         }
> 
> Maxim Khutornenko wrote:
>     I can move the counter in but not the validation check, which goes against the IJobKey.

Why not `IJobKey.build(config.getJob())`?  Given that we've already had one non-obvious bug with this code, i think it's prudent to not replicate it.


- Bill


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


On Oct. 28, 2014, 6:01 p.m., Maxim Khutornenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27262/
> -----------------------------------------------------------
> 
> (Updated Oct. 28, 2014, 6:01 p.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Bugs: AURORA-899
>     https://issues.apache.org/jira/browse/AURORA-899
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Adding missing JobConfiguration backfill for task config job field.
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java 97517ed609289f584e07620d24f6fc09c54d36fc 
>   src/test/java/org/apache/aurora/scheduler/storage/StorageBackfillTest.java 32a90d5a7819404a3d9e0b94168a447398607e56 
> 
> Diff: https://reviews.apache.org/r/27262/diff/
> 
> 
> Testing
> -------
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>


Re: Review Request 27262: Adding missing JobConfiguration backfill for task config job field.

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

> On Oct. 28, 2014, 5:30 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java, line 129
> > <https://reviews.apache.org/r/27262/diff/2/?file=735497#file735497line129>
> >
> >     Can you fully centralize the backfill and the counter increment?
> >     
> >         void populateJobKey(TaskConfig config) {
> >           if job key is valid:
> >             fill job key
> >             increment counter
> >         }
> 
> Maxim Khutornenko wrote:
>     I can move the counter in but not the validation check, which goes against the IJobKey.
> 
> Bill Farner wrote:
>     Why not `IJobKey.build(config.getJob())`?  Given that we've already had one non-obvious bug with this code, i think it's prudent to not replicate it.

That's not going to work if `config.getJob()` is NULL.


- Maxim


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


On Oct. 28, 2014, 6:01 p.m., Maxim Khutornenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27262/
> -----------------------------------------------------------
> 
> (Updated Oct. 28, 2014, 6:01 p.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Bugs: AURORA-899
>     https://issues.apache.org/jira/browse/AURORA-899
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Adding missing JobConfiguration backfill for task config job field.
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java 97517ed609289f584e07620d24f6fc09c54d36fc 
>   src/test/java/org/apache/aurora/scheduler/storage/StorageBackfillTest.java 32a90d5a7819404a3d9e0b94168a447398607e56 
> 
> Diff: https://reviews.apache.org/r/27262/diff/
> 
> 
> Testing
> -------
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>


Re: Review Request 27262: Adding missing JobConfiguration backfill for task config job field.

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

> On Oct. 28, 2014, 5:30 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java, line 129
> > <https://reviews.apache.org/r/27262/diff/2/?file=735497#file735497line129>
> >
> >     Can you fully centralize the backfill and the counter increment?
> >     
> >         void populateJobKey(TaskConfig config) {
> >           if job key is valid:
> >             fill job key
> >             increment counter
> >         }

I can move the counter in but not the validation check, which goes against the IJobKey.


- Maxim


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


On Oct. 28, 2014, 5:22 p.m., Maxim Khutornenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27262/
> -----------------------------------------------------------
> 
> (Updated Oct. 28, 2014, 5:22 p.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Bugs: AURORA-899
>     https://issues.apache.org/jira/browse/AURORA-899
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Adding missing JobConfiguration backfill for task config job field.
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java 97517ed609289f584e07620d24f6fc09c54d36fc 
>   src/test/java/org/apache/aurora/scheduler/storage/StorageBackfillTest.java 32a90d5a7819404a3d9e0b94168a447398607e56 
> 
> Diff: https://reviews.apache.org/r/27262/diff/
> 
> 
> Testing
> -------
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>


Re: Review Request 27262: Adding missing JobConfiguration backfill for task config job field.

Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27262/#review58828
-----------------------------------------------------------



src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java
<https://reviews.apache.org/r/27262/#comment99987>

    Can you fully centralize the backfill and the counter increment?
    
        void populateJobKey(TaskConfig config) {
          if job key is valid:
            fill job key
            increment counter
        }


- Bill Farner


On Oct. 28, 2014, 5:22 p.m., Maxim Khutornenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27262/
> -----------------------------------------------------------
> 
> (Updated Oct. 28, 2014, 5:22 p.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Bugs: AURORA-899
>     https://issues.apache.org/jira/browse/AURORA-899
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Adding missing JobConfiguration backfill for task config job field.
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java 97517ed609289f584e07620d24f6fc09c54d36fc 
>   src/test/java/org/apache/aurora/scheduler/storage/StorageBackfillTest.java 32a90d5a7819404a3d9e0b94168a447398607e56 
> 
> Diff: https://reviews.apache.org/r/27262/diff/
> 
> 
> Testing
> -------
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>


Re: Review Request 27262: Adding missing JobConfiguration backfill for task config job field.

Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27262/#review58832
-----------------------------------------------------------


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

  pants build (options) [spec] (build args)
  pants build (options) [spec]... -- (build args)

Options:
  --version             show program's version number and exit
  --no-pantsrc          Specifies that pantsrc files should be ignored.
  --log-exit            Log an exit message on success or failure.
  -t CONN_TIMEOUT, --timeout=CONN_TIMEOUT
                        Number of seconds to wait for http connections.
  -i INTERPRETERS, --interpreter=INTERPRETERS
                        Constrain what Python interpreters to use.  Uses
                        Requirement format from pkg_resources, e.g.
                        'CPython>=2.6,<3' or 'PyPy'. By default, no
                        constraints are used.  Multiple constraints may be
                        added.  They will be ORed together.
  -v, --verbose         Show verbose output.
  -f, --fast            Run tests in a single chroot.

Builds the specified Python target(s). Use ./pants goal for JVM and other
targets.

- Aurora ReviewBot


On Oct. 28, 2014, 5:22 p.m., Maxim Khutornenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27262/
> -----------------------------------------------------------
> 
> (Updated Oct. 28, 2014, 5:22 p.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Bugs: AURORA-899
>     https://issues.apache.org/jira/browse/AURORA-899
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Adding missing JobConfiguration backfill for task config job field.
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java 97517ed609289f584e07620d24f6fc09c54d36fc 
>   src/test/java/org/apache/aurora/scheduler/storage/StorageBackfillTest.java 32a90d5a7819404a3d9e0b94168a447398607e56 
> 
> Diff: https://reviews.apache.org/r/27262/diff/
> 
> 
> Testing
> -------
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>


Re: Review Request 27262: Adding missing JobConfiguration backfill for task config job field.

Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27262/#review58860
-----------------------------------------------------------

Ship it!


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

- Aurora ReviewBot


On Oct. 28, 2014, 8:26 p.m., Maxim Khutornenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27262/
> -----------------------------------------------------------
> 
> (Updated Oct. 28, 2014, 8:26 p.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Bugs: AURORA-899
>     https://issues.apache.org/jira/browse/AURORA-899
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Adding missing JobConfiguration backfill for task config job field.
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java 97517ed609289f584e07620d24f6fc09c54d36fc 
>   src/test/java/org/apache/aurora/scheduler/storage/StorageBackfillTest.java 32a90d5a7819404a3d9e0b94168a447398607e56 
> 
> Diff: https://reviews.apache.org/r/27262/diff/
> 
> 
> Testing
> -------
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>


Re: Review Request 27262: Adding missing JobConfiguration backfill for task config job field.

Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27262/#review58861
-----------------------------------------------------------

Ship it!


- Bill Farner


On Oct. 28, 2014, 8:26 p.m., Maxim Khutornenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27262/
> -----------------------------------------------------------
> 
> (Updated Oct. 28, 2014, 8:26 p.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Bugs: AURORA-899
>     https://issues.apache.org/jira/browse/AURORA-899
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Adding missing JobConfiguration backfill for task config job field.
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java 97517ed609289f584e07620d24f6fc09c54d36fc 
>   src/test/java/org/apache/aurora/scheduler/storage/StorageBackfillTest.java 32a90d5a7819404a3d9e0b94168a447398607e56 
> 
> Diff: https://reviews.apache.org/r/27262/diff/
> 
> 
> Testing
> -------
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>


Re: Review Request 27262: Adding missing JobConfiguration backfill for task config job field.

Posted by Maxim Khutornenko <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27262/
-----------------------------------------------------------

(Updated Oct. 28, 2014, 8:26 p.m.)


Review request for Aurora and Bill Farner.


Changes
-------

Addressed CR comments.


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


Repository: aurora


Description
-------

Adding missing JobConfiguration backfill for task config job field.


Diffs (updated)
-----

  src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java 97517ed609289f584e07620d24f6fc09c54d36fc 
  src/test/java/org/apache/aurora/scheduler/storage/StorageBackfillTest.java 32a90d5a7819404a3d9e0b94168a447398607e56 

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


Testing
-------

./gradlew -Pq build


Thanks,

Maxim Khutornenko


Re: Review Request 27262: Adding missing JobConfiguration backfill for task config job field.

Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27262/#review58843
-----------------------------------------------------------

Ship it!


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

- Aurora ReviewBot


On Oct. 28, 2014, 6:01 p.m., Maxim Khutornenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27262/
> -----------------------------------------------------------
> 
> (Updated Oct. 28, 2014, 6:01 p.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Bugs: AURORA-899
>     https://issues.apache.org/jira/browse/AURORA-899
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Adding missing JobConfiguration backfill for task config job field.
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java 97517ed609289f584e07620d24f6fc09c54d36fc 
>   src/test/java/org/apache/aurora/scheduler/storage/StorageBackfillTest.java 32a90d5a7819404a3d9e0b94168a447398607e56 
> 
> Diff: https://reviews.apache.org/r/27262/diff/
> 
> 
> Testing
> -------
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>


Re: Review Request 27262: Adding missing JobConfiguration backfill for task config job field.

Posted by Maxim Khutornenko <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27262/
-----------------------------------------------------------

(Updated Oct. 28, 2014, 6:01 p.m.)


Review request for Aurora and Bill Farner.


Changes
-------

CR comments.


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


Repository: aurora


Description
-------

Adding missing JobConfiguration backfill for task config job field.


Diffs (updated)
-----

  src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java 97517ed609289f584e07620d24f6fc09c54d36fc 
  src/test/java/org/apache/aurora/scheduler/storage/StorageBackfillTest.java 32a90d5a7819404a3d9e0b94168a447398607e56 

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


Testing
-------

./gradlew -Pq build


Thanks,

Maxim Khutornenko


Re: Review Request 27262: Adding missing JobConfiguration backfill for task config job field.

Posted by Maxim Khutornenko <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27262/
-----------------------------------------------------------

(Updated Oct. 28, 2014, 5:22 p.m.)


Review request for Aurora and Bill Farner.


Changes
-------

CR comments.


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


Repository: aurora


Description
-------

Adding missing JobConfiguration backfill for task config job field.


Diffs (updated)
-----

  src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java 97517ed609289f584e07620d24f6fc09c54d36fc 
  src/test/java/org/apache/aurora/scheduler/storage/StorageBackfillTest.java 32a90d5a7819404a3d9e0b94168a447398607e56 

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


Testing
-------

./gradlew -Pq build


Thanks,

Maxim Khutornenko


Re: Review Request 27262: Adding missing JobConfiguration backfill for task config job field.

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

> On Oct. 28, 2014, 5:04 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java, line 67
> > <https://reviews.apache.org/r/27262/diff/1/?file=734815#file734815line67>
> >
> >     Is the cargo cult of the code below necessary?

I was hestitant to invest more given a throwaway nature of this change. Refactored.


- Maxim


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


On Oct. 28, 2014, 1:43 a.m., Maxim Khutornenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27262/
> -----------------------------------------------------------
> 
> (Updated Oct. 28, 2014, 1:43 a.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Bugs: AURORA-899
>     https://issues.apache.org/jira/browse/AURORA-899
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Adding missing JobConfiguration backfill for task config job field.
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java 97517ed609289f584e07620d24f6fc09c54d36fc 
>   src/test/java/org/apache/aurora/scheduler/storage/StorageBackfillTest.java 32a90d5a7819404a3d9e0b94168a447398607e56 
> 
> Diff: https://reviews.apache.org/r/27262/diff/
> 
> 
> Testing
> -------
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>


Re: Review Request 27262: Adding missing JobConfiguration backfill for task config job field.

Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27262/#review58825
-----------------------------------------------------------



src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java
<https://reviews.apache.org/r/27262/#comment99984>

    Is the cargo cult of the code below necessary?


- Bill Farner


On Oct. 28, 2014, 1:43 a.m., Maxim Khutornenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27262/
> -----------------------------------------------------------
> 
> (Updated Oct. 28, 2014, 1:43 a.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Bugs: AURORA-899
>     https://issues.apache.org/jira/browse/AURORA-899
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Adding missing JobConfiguration backfill for task config job field.
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java 97517ed609289f584e07620d24f6fc09c54d36fc 
>   src/test/java/org/apache/aurora/scheduler/storage/StorageBackfillTest.java 32a90d5a7819404a3d9e0b94168a447398607e56 
> 
> Diff: https://reviews.apache.org/r/27262/diff/
> 
> 
> Testing
> -------
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>