Re: Review Request 26954: Preparing for Identity struct deprecation (client and executor).

[Maxim Khutornenko]

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

(Updated Oct. 23, 2014, 10:58 p.m.)


Review request for Aurora, Kevin Sweeney and Bill Farner.


Changes
---

Rebased


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


Repository: aurora


Description
---

Python side of changes for deprecating identity struct. Accounting for the 
upcoming deprecation by backfilling missing fields and switching between 
old/new fields (whichever is available).

Branched off of https://reviews.apache.org/r/26762/ and will have to be pushed 
together.

Summary of changes:
- TaskConfig - backfilling _key_ field in job diff and update commands.

Also, some unit test refactoring to avoid copy-pasted fragments.


Diffs (updated)
-

  src/main/python/apache/aurora/client/api/instance_watcher.py 
b390aa8993205f1f6938f8c295e3c16a0bf4df6d 
  src/main/python/apache/aurora/client/api/sla.py 
b9b64680b15f5395ed6aca681b9b1c30ffe2822c 
  src/main/python/apache/aurora/client/cli/jobs.py 
493ba21bd54768412ea3d293b6f0ccafada8158a 
  src/main/python/apache/aurora/client/cli/task.py 
c41484bdc27266443bc4e139e1ebb362a59be0f9 
  src/main/python/apache/aurora/client/commands/admin.py 
deee0250f3ba9837feeb92acc654f5b3b68b4e0f 
  src/main/python/apache/aurora/client/commands/core.py 
58f419e674f1a9a0ae9da6faa2e39c8167bab597 
  src/main/python/apache/aurora/client/commands/ssh.py 
d2b8bf675556b924d3d63b545d036dc48a081486 
  src/main/python/apache/aurora/config/thrift.py 
9ca806db6d2c59836196ea2fdd3fc015b9f6b71f 
  src/main/python/apache/aurora/executor/aurora_executor.py 
2c6423d096656f426a4385f4edef6875ebad7049 
  src/main/python/apache/aurora/executor/common/announcer.py 
74b2114d1ede7a4a4a68b78144f9b08bc994bd87 
  src/main/python/apache/aurora/executor/thermos_task_runner.py 
bb99bd12d7387e080d8d4c0557d2afa3bf5e7c60 
  src/test/python/apache/aurora/client/api/test_instance_watcher.py 
ae1b24bf4e3291cb31b3129cabcacdf32db0c560 
  src/test/python/apache/aurora/client/api/test_sla.py 
1117f24d5ad3640632a1dd728913ba73c8bec707 
  src/test/python/apache/aurora/client/cli/test_api_from_cli.py 
95aa649cfff9166dd10aa432c4d470739e8f06c5 
  src/test/python/apache/aurora/client/cli/test_diff.py 
10817695352687cdb5b0c3ed9720e3091b230e68 
  src/test/python/apache/aurora/client/cli/test_status.py 
49ac2a46b4ae2546f7012915076b6659f42e3100 
  src/test/python/apache/aurora/client/cli/test_task_run.py 
16fde14c03f6fd2c000e76625fad174835763f1b 
  src/test/python/apache/aurora/client/cli/test_update.py 
1ec5483506a22a774340acccd33f09f1742be8b7 
  src/test/python/apache/aurora/client/cli/util.py 
3fa609a5f71525393ca0a5dbd81423005fadb583 
  src/test/python/apache/aurora/client/commands/test_diff.py 
c8d01456aa52fd61374b4f0960b5159da2cb235b 
  src/test/python/apache/aurora/client/commands/test_ssh.py 
abb657ba397c23ddac6c6b188f70d1c4e34597a6 
  src/test/python/apache/aurora/client/commands/test_status.py 
639763501348a35bff2f127e18780ac74852f51b 
  src/test/python/apache/aurora/client/commands/test_update.py 
07cbe53109e8bcdd09dcac47f6353b10e095717d 
  src/test/python/apache/aurora/config/test_thrift.py 
1dd9e79099793aa7bb377c20c2a6a0c4b750be33 
  src/test/python/apache/aurora/executor/common/test_announcer.py 
56943351ca09c29580dd764bb2442f0fcd9fde74 
  src/test/python/apache/aurora/executor/test_thermos_executor.py 
65e8cce60a5543081175c574aaaf92f200bc6233 

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


Testing
---

./pants src/test/python:all
test_end_to_end.sh


Thanks,

Maxim Khutornenko

Re: Review Request 26954: Preparing for Identity struct deprecation (client and executor).

[Kevin Sweeney]

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

Ship it!


Ship It!

- Kevin Sweeney


On Oct. 23, 2014, 3:02 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26954/
> ---
> 
> (Updated Oct. 23, 2014, 3:02 p.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Bill Farner.
> 
> 
> Bugs: AURORA-84
> https://issues.apache.org/jira/browse/AURORA-84
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Python side of changes for deprecating identity struct. Accounting for the 
> upcoming deprecation by backfilling missing fields and switching between 
> old/new fields (whichever is available).
> 
> Branched off of https://reviews.apache.org/r/26762/ and will have to be 
> pushed together.
> 
> Summary of changes:
> - TaskConfig - backfilling _key_ field in job diff and update commands.
> 
> Also, some unit test refactoring to avoid copy-pasted fragments.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/client/api/instance_watcher.py 
> b390aa8993205f1f6938f8c295e3c16a0bf4df6d 
>   src/main/python/apache/aurora/client/api/sla.py 
> b9b64680b15f5395ed6aca681b9b1c30ffe2822c 
>   src/main/python/apache/aurora/client/cli/jobs.py 
> 10f8e0d331ca607e55e3aa6f96014caea744ed9f 
>   src/main/python/apache/aurora/client/cli/task.py 
> c41484bdc27266443bc4e139e1ebb362a59be0f9 
>   src/main/python/apache/aurora/client/commands/admin.py 
> deee0250f3ba9837feeb92acc654f5b3b68b4e0f 
>   src/main/python/apache/aurora/client/commands/core.py 
> 58f419e674f1a9a0ae9da6faa2e39c8167bab597 
>   src/main/python/apache/aurora/client/commands/ssh.py 
> d2b8bf675556b924d3d63b545d036dc48a081486 
>   src/main/python/apache/aurora/config/thrift.py 
> 9ca806db6d2c59836196ea2fdd3fc015b9f6b71f 
>   src/main/python/apache/aurora/executor/aurora_executor.py 
> 2c6423d096656f426a4385f4edef6875ebad7049 
>   src/main/python/apache/aurora/executor/common/announcer.py 
> 74b2114d1ede7a4a4a68b78144f9b08bc994bd87 
>   src/main/python/apache/aurora/executor/thermos_task_runner.py 
> bb99bd12d7387e080d8d4c0557d2afa3bf5e7c60 
>   src/test/python/apache/aurora/client/api/test_instance_watcher.py 
> ae1b24bf4e3291cb31b3129cabcacdf32db0c560 
>   src/test/python/apache/aurora/client/api/test_sla.py 
> 1117f24d5ad3640632a1dd728913ba73c8bec707 
>   src/test/python/apache/aurora/client/cli/test_api_from_cli.py 
> 95aa649cfff9166dd10aa432c4d470739e8f06c5 
>   src/test/python/apache/aurora/client/cli/test_diff.py 
> 10817695352687cdb5b0c3ed9720e3091b230e68 
>   src/test/python/apache/aurora/client/cli/test_status.py 
> 4f62cf0c52e5837309cf7ad702df6d907df8f510 
>   src/test/python/apache/aurora/client/cli/test_task_run.py 
> 16fde14c03f6fd2c000e76625fad174835763f1b 
>   src/test/python/apache/aurora/client/cli/test_update.py 
> 1ec5483506a22a774340acccd33f09f1742be8b7 
>   src/test/python/apache/aurora/client/cli/util.py 
> 3fa609a5f71525393ca0a5dbd81423005fadb583 
>   src/test/python/apache/aurora/client/commands/test_diff.py 
> c8d01456aa52fd61374b4f0960b5159da2cb235b 
>   src/test/python/apache/aurora/client/commands/test_ssh.py 
> abb657ba397c23ddac6c6b188f70d1c4e34597a6 
>   src/test/python/apache/aurora/client/commands/test_status.py 
> 639763501348a35bff2f127e18780ac74852f51b 
>   src/test/python/apache/aurora/client/commands/test_update.py 
> 07cbe53109e8bcdd09dcac47f6353b10e095717d 
>   src/test/python/apache/aurora/config/test_thrift.py 
> 1dd9e79099793aa7bb377c20c2a6a0c4b750be33 
>   src/test/python/apache/aurora/executor/common/test_announcer.py 
> 56943351ca09c29580dd764bb2442f0fcd9fde74 
>   src/test/python/apache/aurora/executor/test_thermos_executor.py 
> 65e8cce60a5543081175c574aaaf92f200bc6233 
> 
> Diff: https://reviews.apache.org/r/26954/diff/
> 
> 
> Testing
> ---
> 
> ./pants src/test/python:all
> test_end_to_end.sh
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 26954: Preparing for Identity struct deprecation (client and executor).

[Bill Farner]

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

Ship it!


Ship It!

- Bill Farner


On Oct. 23, 2014, 10:02 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26954/
> ---
> 
> (Updated Oct. 23, 2014, 10:02 p.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Bill Farner.
> 
> 
> Bugs: AURORA-84
> https://issues.apache.org/jira/browse/AURORA-84
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Python side of changes for deprecating identity struct. Accounting for the 
> upcoming deprecation by backfilling missing fields and switching between 
> old/new fields (whichever is available).
> 
> Branched off of https://reviews.apache.org/r/26762/ and will have to be 
> pushed together.
> 
> Summary of changes:
> - TaskConfig - backfilling _key_ field in job diff and update commands.
> 
> Also, some unit test refactoring to avoid copy-pasted fragments.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/client/api/instance_watcher.py 
> b390aa8993205f1f6938f8c295e3c16a0bf4df6d 
>   src/main/python/apache/aurora/client/api/sla.py 
> b9b64680b15f5395ed6aca681b9b1c30ffe2822c 
>   src/main/python/apache/aurora/client/cli/jobs.py 
> 10f8e0d331ca607e55e3aa6f96014caea744ed9f 
>   src/main/python/apache/aurora/client/cli/task.py 
> c41484bdc27266443bc4e139e1ebb362a59be0f9 
>   src/main/python/apache/aurora/client/commands/admin.py 
> deee0250f3ba9837feeb92acc654f5b3b68b4e0f 
>   src/main/python/apache/aurora/client/commands/core.py 
> 58f419e674f1a9a0ae9da6faa2e39c8167bab597 
>   src/main/python/apache/aurora/client/commands/ssh.py 
> d2b8bf675556b924d3d63b545d036dc48a081486 
>   src/main/python/apache/aurora/config/thrift.py 
> 9ca806db6d2c59836196ea2fdd3fc015b9f6b71f 
>   src/main/python/apache/aurora/executor/aurora_executor.py 
> 2c6423d096656f426a4385f4edef6875ebad7049 
>   src/main/python/apache/aurora/executor/common/announcer.py 
> 74b2114d1ede7a4a4a68b78144f9b08bc994bd87 
>   src/main/python/apache/aurora/executor/thermos_task_runner.py 
> bb99bd12d7387e080d8d4c0557d2afa3bf5e7c60 
>   src/test/python/apache/aurora/client/api/test_instance_watcher.py 
> ae1b24bf4e3291cb31b3129cabcacdf32db0c560 
>   src/test/python/apache/aurora/client/api/test_sla.py 
> 1117f24d5ad3640632a1dd728913ba73c8bec707 
>   src/test/python/apache/aurora/client/cli/test_api_from_cli.py 
> 95aa649cfff9166dd10aa432c4d470739e8f06c5 
>   src/test/python/apache/aurora/client/cli/test_diff.py 
> 10817695352687cdb5b0c3ed9720e3091b230e68 
>   src/test/python/apache/aurora/client/cli/test_status.py 
> 4f62cf0c52e5837309cf7ad702df6d907df8f510 
>   src/test/python/apache/aurora/client/cli/test_task_run.py 
> 16fde14c03f6fd2c000e76625fad174835763f1b 
>   src/test/python/apache/aurora/client/cli/test_update.py 
> 1ec5483506a22a774340acccd33f09f1742be8b7 
>   src/test/python/apache/aurora/client/cli/util.py 
> 3fa609a5f71525393ca0a5dbd81423005fadb583 
>   src/test/python/apache/aurora/client/commands/test_diff.py 
> c8d01456aa52fd61374b4f0960b5159da2cb235b 
>   src/test/python/apache/aurora/client/commands/test_ssh.py 
> abb657ba397c23ddac6c6b188f70d1c4e34597a6 
>   src/test/python/apache/aurora/client/commands/test_status.py 
> 639763501348a35bff2f127e18780ac74852f51b 
>   src/test/python/apache/aurora/client/commands/test_update.py 
> 07cbe53109e8bcdd09dcac47f6353b10e095717d 
>   src/test/python/apache/aurora/config/test_thrift.py 
> 1dd9e79099793aa7bb377c20c2a6a0c4b750be33 
>   src/test/python/apache/aurora/executor/common/test_announcer.py 
> 56943351ca09c29580dd764bb2442f0fcd9fde74 
>   src/test/python/apache/aurora/executor/test_thermos_executor.py 
> 65e8cce60a5543081175c574aaaf92f200bc6233 
> 
> Diff: https://reviews.apache.org/r/26954/diff/
> 
> 
> Testing
> ---
> 
> ./pants src/test/python:all
> test_end_to_end.sh
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 26954: Preparing for Identity struct deprecation (client and executor).

[Maxim Khutornenko]

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

(Updated Oct. 23, 2014, 10:02 p.m.)


Review request for Aurora, Kevin Sweeney and Bill Farner.


Changes
---

Fixed status command changes escaped rebasing.


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


Repository: aurora


Description
---

Python side of changes for deprecating identity struct. Accounting for the 
upcoming deprecation by backfilling missing fields and switching between 
old/new fields (whichever is available).

Branched off of https://reviews.apache.org/r/26762/ and will have to be pushed 
together.

Summary of changes:
- TaskConfig - backfilling _key_ field in job diff and update commands.

Also, some unit test refactoring to avoid copy-pasted fragments.


Diffs (updated)
-

  src/main/python/apache/aurora/client/api/instance_watcher.py 
b390aa8993205f1f6938f8c295e3c16a0bf4df6d 
  src/main/python/apache/aurora/client/api/sla.py 
b9b64680b15f5395ed6aca681b9b1c30ffe2822c 
  src/main/python/apache/aurora/client/cli/jobs.py 
10f8e0d331ca607e55e3aa6f96014caea744ed9f 
  src/main/python/apache/aurora/client/cli/task.py 
c41484bdc27266443bc4e139e1ebb362a59be0f9 
  src/main/python/apache/aurora/client/commands/admin.py 
deee0250f3ba9837feeb92acc654f5b3b68b4e0f 
  src/main/python/apache/aurora/client/commands/core.py 
58f419e674f1a9a0ae9da6faa2e39c8167bab597 
  src/main/python/apache/aurora/client/commands/ssh.py 
d2b8bf675556b924d3d63b545d036dc48a081486 
  src/main/python/apache/aurora/config/thrift.py 
9ca806db6d2c59836196ea2fdd3fc015b9f6b71f 
  src/main/python/apache/aurora/executor/aurora_executor.py 
2c6423d096656f426a4385f4edef6875ebad7049 
  src/main/python/apache/aurora/executor/common/announcer.py 
74b2114d1ede7a4a4a68b78144f9b08bc994bd87 
  src/main/python/apache/aurora/executor/thermos_task_runner.py 
bb99bd12d7387e080d8d4c0557d2afa3bf5e7c60 
  src/test/python/apache/aurora/client/api/test_instance_watcher.py 
ae1b24bf4e3291cb31b3129cabcacdf32db0c560 
  src/test/python/apache/aurora/client/api/test_sla.py 
1117f24d5ad3640632a1dd728913ba73c8bec707 
  src/test/python/apache/aurora/client/cli/test_api_from_cli.py 
95aa649cfff9166dd10aa432c4d470739e8f06c5 
  src/test/python/apache/aurora/client/cli/test_diff.py 
10817695352687cdb5b0c3ed9720e3091b230e68 
  src/test/python/apache/aurora/client/cli/test_status.py 
4f62cf0c52e5837309cf7ad702df6d907df8f510 
  src/test/python/apache/aurora/client/cli/test_task_run.py 
16fde14c03f6fd2c000e76625fad174835763f1b 
  src/test/python/apache/aurora/client/cli/test_update.py 
1ec5483506a22a774340acccd33f09f1742be8b7 
  src/test/python/apache/aurora/client/cli/util.py 
3fa609a5f71525393ca0a5dbd81423005fadb583 
  src/test/python/apache/aurora/client/commands/test_diff.py 
c8d01456aa52fd61374b4f0960b5159da2cb235b 
  src/test/python/apache/aurora/client/commands/test_ssh.py 
abb657ba397c23ddac6c6b188f70d1c4e34597a6 
  src/test/python/apache/aurora/client/commands/test_status.py 
639763501348a35bff2f127e18780ac74852f51b 
  src/test/python/apache/aurora/client/commands/test_update.py 
07cbe53109e8bcdd09dcac47f6353b10e095717d 
  src/test/python/apache/aurora/config/test_thrift.py 
1dd9e79099793aa7bb377c20c2a6a0c4b750be33 
  src/test/python/apache/aurora/executor/common/test_announcer.py 
56943351ca09c29580dd764bb2442f0fcd9fde74 
  src/test/python/apache/aurora/executor/test_thermos_executor.py 
65e8cce60a5543081175c574aaaf92f200bc6233 

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


Testing
---

./pants src/test/python:all
test_end_to_end.sh


Thanks,

Maxim Khutornenko

Re: Review Request 26954: Preparing for Identity struct deprecation (client and executor).

[Maxim Khutornenko]

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

(Updated Oct. 23, 2014, 8:40 p.m.)


Review request for Aurora, Kevin Sweeney and Bill Farner.


Changes
---

Rebased and addressed comments.


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


Repository: aurora


Description
---

Python side of changes for deprecating identity struct. Accounting for the 
upcoming deprecation by backfilling missing fields and switching between 
old/new fields (whichever is available).

Branched off of https://reviews.apache.org/r/26762/ and will have to be pushed 
together.

Summary of changes:
- TaskConfig - backfilling _key_ field in job diff and update commands.

Also, some unit test refactoring to avoid copy-pasted fragments.


Diffs (updated)
-

  src/main/python/apache/aurora/client/api/instance_watcher.py 
b390aa8993205f1f6938f8c295e3c16a0bf4df6d 
  src/main/python/apache/aurora/client/api/sla.py 
b9b64680b15f5395ed6aca681b9b1c30ffe2822c 
  src/main/python/apache/aurora/client/cli/task.py 
c41484bdc27266443bc4e139e1ebb362a59be0f9 
  src/main/python/apache/aurora/client/commands/admin.py 
deee0250f3ba9837feeb92acc654f5b3b68b4e0f 
  src/main/python/apache/aurora/client/commands/core.py 
58f419e674f1a9a0ae9da6faa2e39c8167bab597 
  src/main/python/apache/aurora/client/commands/ssh.py 
d2b8bf675556b924d3d63b545d036dc48a081486 
  src/main/python/apache/aurora/config/thrift.py 
9ca806db6d2c59836196ea2fdd3fc015b9f6b71f 
  src/main/python/apache/aurora/executor/aurora_executor.py 
2c6423d096656f426a4385f4edef6875ebad7049 
  src/main/python/apache/aurora/executor/common/announcer.py 
74b2114d1ede7a4a4a68b78144f9b08bc994bd87 
  src/main/python/apache/aurora/executor/thermos_task_runner.py 
bb99bd12d7387e080d8d4c0557d2afa3bf5e7c60 
  src/test/python/apache/aurora/client/api/test_instance_watcher.py 
ae1b24bf4e3291cb31b3129cabcacdf32db0c560 
  src/test/python/apache/aurora/client/api/test_sla.py 
1117f24d5ad3640632a1dd728913ba73c8bec707 
  src/test/python/apache/aurora/client/cli/test_api_from_cli.py 
95aa649cfff9166dd10aa432c4d470739e8f06c5 
  src/test/python/apache/aurora/client/cli/test_diff.py 
10817695352687cdb5b0c3ed9720e3091b230e68 
  src/test/python/apache/aurora/client/cli/test_status.py 
4f62cf0c52e5837309cf7ad702df6d907df8f510 
  src/test/python/apache/aurora/client/cli/test_task_run.py 
16fde14c03f6fd2c000e76625fad174835763f1b 
  src/test/python/apache/aurora/client/cli/test_update.py 
1ec5483506a22a774340acccd33f09f1742be8b7 
  src/test/python/apache/aurora/client/cli/util.py 
3fa609a5f71525393ca0a5dbd81423005fadb583 
  src/test/python/apache/aurora/client/commands/test_diff.py 
c8d01456aa52fd61374b4f0960b5159da2cb235b 
  src/test/python/apache/aurora/client/commands/test_ssh.py 
abb657ba397c23ddac6c6b188f70d1c4e34597a6 
  src/test/python/apache/aurora/client/commands/test_status.py 
639763501348a35bff2f127e18780ac74852f51b 
  src/test/python/apache/aurora/client/commands/test_update.py 
07cbe53109e8bcdd09dcac47f6353b10e095717d 
  src/test/python/apache/aurora/config/test_thrift.py 
1dd9e79099793aa7bb377c20c2a6a0c4b750be33 
  src/test/python/apache/aurora/executor/common/test_announcer.py 
56943351ca09c29580dd764bb2442f0fcd9fde74 
  src/test/python/apache/aurora/executor/test_thermos_executor.py 
65e8cce60a5543081175c574aaaf92f200bc6233 

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


Testing
---

./pants src/test/python:all
test_end_to_end.sh


Thanks,

Maxim Khutornenko

Re: Review Request 26954: Preparing for Identity struct deprecation (client and executor).

[Maxim Khutornenko]

> On Oct. 22, 2014, 11:51 p.m., Bill Farner wrote:
> > src/test/python/apache/aurora/client/cli/test_diff.py, line 60
> > 
> >
> > While you're here, please change this to not use Mock, all down the 
> > hierarchy.
> > 
> > Ditto for other cargo cults (thanks for collapsing a few, btw).
> 
> Maxim Khutornenko wrote:
> I collapsed all related cli methods into util.py and using specs for all 
> Mocks now. Did not modify the commands (v1) tests to let them die out 
> naturally.
> 
> I actually like mocks here as tests fail when accessing a field that is 
> not populated.
> 
> Bill Farner wrote:
> Please chime in on https://reviews.apache.org/r/27058/, where the exact 
> opposite perspective exists.
> 
> Maxim Khutornenko wrote:
> Commented there. Having a Mock with spec here really helps identifying 
> the usage.
> 
> Kevin Sweeney wrote:
> See my comments there - tl;dr `Mock(spec=ThriftStruct)` should be 
> considered harmful as it's no better than `Mock(spec=object)`.
> 
> Either calling the kwargs constructor or using `spec_set` with the 
> constructor kwargs retrieved via reflection gets us the correct behavior 
> here. Since this seems to be a source of confusion for several people we're 
> probably well-served factoring out a thrift_test_util library or moving to 
> kwargs everywhere.

Agree, spec is not deterministic enough in this case. Refactored.


- Maxim


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


On Oct. 22, 2014, 11:59 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26954/
> ---
> 
> (Updated Oct. 22, 2014, 11:59 p.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Bill Farner.
> 
> 
> Bugs: AURORA-84
> https://issues.apache.org/jira/browse/AURORA-84
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Python side of changes for deprecating identity struct. Accounting for the 
> upcoming deprecation by backfilling missing fields and switching between 
> old/new fields (whichever is available).
> 
> Branched off of https://reviews.apache.org/r/26762/ and will have to be 
> pushed together.
> 
> Summary of changes:
> - TaskConfig - backfilling _key_ field in job diff and update commands.
> 
> Also, some unit test refactoring to avoid copy-pasted fragments.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/client/api/instance_watcher.py 
> b390aa8993205f1f6938f8c295e3c16a0bf4df6d 
>   src/main/python/apache/aurora/client/api/sla.py 
> b9b64680b15f5395ed6aca681b9b1c30ffe2822c 
>   src/main/python/apache/aurora/client/cli/task.py 
> c41484bdc27266443bc4e139e1ebb362a59be0f9 
>   src/main/python/apache/aurora/client/commands/admin.py 
> deee0250f3ba9837feeb92acc654f5b3b68b4e0f 
>   src/main/python/apache/aurora/client/commands/core.py 
> 58f419e674f1a9a0ae9da6faa2e39c8167bab597 
>   src/main/python/apache/aurora/client/commands/ssh.py 
> d2b8bf675556b924d3d63b545d036dc48a081486 
>   src/main/python/apache/aurora/config/thrift.py 
> 9ca806db6d2c59836196ea2fdd3fc015b9f6b71f 
>   src/main/python/apache/aurora/executor/aurora_executor.py 
> 2c6423d096656f426a4385f4edef6875ebad7049 
>   src/main/python/apache/aurora/executor/common/announcer.py 
> 74b2114d1ede7a4a4a68b78144f9b08bc994bd87 
>   src/main/python/apache/aurora/executor/thermos_task_runner.py 
> bb99bd12d7387e080d8d4c0557d2afa3bf5e7c60 
>   src/test/python/apache/aurora/client/api/test_instance_watcher.py 
> ae1b24bf4e3291cb31b3129cabcacdf32db0c560 
>   src/test/python/apache/aurora/client/api/test_sla.py 
> 1117f24d5ad3640632a1dd728913ba73c8bec707 
>   src/test/python/apache/aurora/client/cli/test_api_from_cli.py 
> 95aa649cfff9166dd10aa432c4d470739e8f06c5 
>   src/test/python/apache/aurora/client/cli/test_diff.py 
> 10817695352687cdb5b0c3ed9720e3091b230e68 
>   src/test/python/apache/aurora/client/cli/test_status.py 
> 4f62cf0c52e5837309cf7ad702df6d907df8f510 
>   src/test/python/apache/aurora/client/cli/test_task_run.py 
> 16fde14c03f6fd2c000e76625fad174835763f1b 
>   src/test/python/apache/aurora/client/cli/test_update.py 
> 1ec5483506a22a774340acccd33f09f1742be8b7 
>   src/test/python/apache/aurora/client/cli/util.py 
> 3fa609a5f71525393ca0a5dbd81423005fadb583 
>   src/test/python/apache/aurora/client/commands/test_diff.py 
> c8d01456aa52fd61374b4f0960b5159da2cb235b 
>   src/test/python/apache/aurora/client/commands/test_ssh.py 
> abb657ba397c23ddac6c6b188f70d1c4e34597a6 
>   src/test/python/apache/aurora/client/commands/test_status.py 
> 639763501348a35bff2f127e18780ac74852f51b 
>   src/test/python/apache/aurora/client

Re: Review Request 26954: Preparing for Identity struct deprecation (client and executor).

[Kevin Sweeney]

> On Oct. 22, 2014, 4:51 p.m., Bill Farner wrote:
> > src/test/python/apache/aurora/client/cli/test_diff.py, line 60
> > 
> >
> > While you're here, please change this to not use Mock, all down the 
> > hierarchy.
> > 
> > Ditto for other cargo cults (thanks for collapsing a few, btw).
> 
> Maxim Khutornenko wrote:
> I collapsed all related cli methods into util.py and using specs for all 
> Mocks now. Did not modify the commands (v1) tests to let them die out 
> naturally.
> 
> I actually like mocks here as tests fail when accessing a field that is 
> not populated.
> 
> Bill Farner wrote:
> Please chime in on https://reviews.apache.org/r/27058/, where the exact 
> opposite perspective exists.
> 
> Maxim Khutornenko wrote:
> Commented there. Having a Mock with spec here really helps identifying 
> the usage.

See my comments there - tl;dr `Mock(spec=ThriftStruct)` should be considered 
harmful as it's no better than `Mock(spec=object)`.

Either calling the kwargs constructor or using `spec_set` with the constructor 
kwargs retrieved via reflection gets us the correct behavior here. Since this 
seems to be a source of confusion for several people we're probably well-served 
factoring out a thrift_test_util library or moving to kwargs everywhere.


- Kevin


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


On Oct. 22, 2014, 4:59 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26954/
> ---
> 
> (Updated Oct. 22, 2014, 4:59 p.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Bill Farner.
> 
> 
> Bugs: AURORA-84
> https://issues.apache.org/jira/browse/AURORA-84
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Python side of changes for deprecating identity struct. Accounting for the 
> upcoming deprecation by backfilling missing fields and switching between 
> old/new fields (whichever is available).
> 
> Branched off of https://reviews.apache.org/r/26762/ and will have to be 
> pushed together.
> 
> Summary of changes:
> - TaskConfig - backfilling _key_ field in job diff and update commands.
> 
> Also, some unit test refactoring to avoid copy-pasted fragments.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/client/api/instance_watcher.py 
> b390aa8993205f1f6938f8c295e3c16a0bf4df6d 
>   src/main/python/apache/aurora/client/api/sla.py 
> b9b64680b15f5395ed6aca681b9b1c30ffe2822c 
>   src/main/python/apache/aurora/client/cli/task.py 
> c41484bdc27266443bc4e139e1ebb362a59be0f9 
>   src/main/python/apache/aurora/client/commands/admin.py 
> deee0250f3ba9837feeb92acc654f5b3b68b4e0f 
>   src/main/python/apache/aurora/client/commands/core.py 
> 58f419e674f1a9a0ae9da6faa2e39c8167bab597 
>   src/main/python/apache/aurora/client/commands/ssh.py 
> d2b8bf675556b924d3d63b545d036dc48a081486 
>   src/main/python/apache/aurora/config/thrift.py 
> 9ca806db6d2c59836196ea2fdd3fc015b9f6b71f 
>   src/main/python/apache/aurora/executor/aurora_executor.py 
> 2c6423d096656f426a4385f4edef6875ebad7049 
>   src/main/python/apache/aurora/executor/common/announcer.py 
> 74b2114d1ede7a4a4a68b78144f9b08bc994bd87 
>   src/main/python/apache/aurora/executor/thermos_task_runner.py 
> bb99bd12d7387e080d8d4c0557d2afa3bf5e7c60 
>   src/test/python/apache/aurora/client/api/test_instance_watcher.py 
> ae1b24bf4e3291cb31b3129cabcacdf32db0c560 
>   src/test/python/apache/aurora/client/api/test_sla.py 
> 1117f24d5ad3640632a1dd728913ba73c8bec707 
>   src/test/python/apache/aurora/client/cli/test_api_from_cli.py 
> 95aa649cfff9166dd10aa432c4d470739e8f06c5 
>   src/test/python/apache/aurora/client/cli/test_diff.py 
> 10817695352687cdb5b0c3ed9720e3091b230e68 
>   src/test/python/apache/aurora/client/cli/test_status.py 
> 4f62cf0c52e5837309cf7ad702df6d907df8f510 
>   src/test/python/apache/aurora/client/cli/test_task_run.py 
> 16fde14c03f6fd2c000e76625fad174835763f1b 
>   src/test/python/apache/aurora/client/cli/test_update.py 
> 1ec5483506a22a774340acccd33f09f1742be8b7 
>   src/test/python/apache/aurora/client/cli/util.py 
> 3fa609a5f71525393ca0a5dbd81423005fadb583 
>   src/test/python/apache/aurora/client/commands/test_diff.py 
> c8d01456aa52fd61374b4f0960b5159da2cb235b 
>   src/test/python/apache/aurora/client/commands/test_ssh.py 
> abb657ba397c23ddac6c6b188f70d1c4e34597a6 
>   src/test/python/apache/aurora/client/commands/test_status.py 
> 639763501348a35bff2f127e18780ac74852f51b 
>   src/test/python/apache/aurora/client/commands/test_update.py 
> 07cbe53109e8bcdd09dcac47f6353b10e095717d 
>   src/test/python/apache/aurora/config/test_thrift.p

Re: Review Request 26954: Preparing for Identity struct deprecation (client and executor).

[Bill Farner]

> On Oct. 22, 2014, 11:51 p.m., Bill Farner wrote:
> > src/test/python/apache/aurora/client/cli/test_diff.py, line 60
> > 
> >
> > While you're here, please change this to not use Mock, all down the 
> > hierarchy.
> > 
> > Ditto for other cargo cults (thanks for collapsing a few, btw).
> 
> Maxim Khutornenko wrote:
> I collapsed all related cli methods into util.py and using specs for all 
> Mocks now. Did not modify the commands (v1) tests to let them die out 
> naturally.
> 
> I actually like mocks here as tests fail when accessing a field that is 
> not populated.

Please chime in on https://reviews.apache.org/r/27058/, where the exact 
opposite perspective exists.


- Bill


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


On Oct. 22, 2014, 11:59 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26954/
> ---
> 
> (Updated Oct. 22, 2014, 11:59 p.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Bill Farner.
> 
> 
> Bugs: AURORA-84
> https://issues.apache.org/jira/browse/AURORA-84
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Python side of changes for deprecating identity struct. Accounting for the 
> upcoming deprecation by backfilling missing fields and switching between 
> old/new fields (whichever is available).
> 
> Branched off of https://reviews.apache.org/r/26762/ and will have to be 
> pushed together.
> 
> Summary of changes:
> - TaskConfig - backfilling _key_ field in job diff and update commands.
> 
> Also, some unit test refactoring to avoid copy-pasted fragments.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/client/api/instance_watcher.py 
> b390aa8993205f1f6938f8c295e3c16a0bf4df6d 
>   src/main/python/apache/aurora/client/api/sla.py 
> b9b64680b15f5395ed6aca681b9b1c30ffe2822c 
>   src/main/python/apache/aurora/client/cli/task.py 
> c41484bdc27266443bc4e139e1ebb362a59be0f9 
>   src/main/python/apache/aurora/client/commands/admin.py 
> deee0250f3ba9837feeb92acc654f5b3b68b4e0f 
>   src/main/python/apache/aurora/client/commands/core.py 
> 58f419e674f1a9a0ae9da6faa2e39c8167bab597 
>   src/main/python/apache/aurora/client/commands/ssh.py 
> d2b8bf675556b924d3d63b545d036dc48a081486 
>   src/main/python/apache/aurora/config/thrift.py 
> 9ca806db6d2c59836196ea2fdd3fc015b9f6b71f 
>   src/main/python/apache/aurora/executor/aurora_executor.py 
> 2c6423d096656f426a4385f4edef6875ebad7049 
>   src/main/python/apache/aurora/executor/common/announcer.py 
> 74b2114d1ede7a4a4a68b78144f9b08bc994bd87 
>   src/main/python/apache/aurora/executor/thermos_task_runner.py 
> bb99bd12d7387e080d8d4c0557d2afa3bf5e7c60 
>   src/test/python/apache/aurora/client/api/test_instance_watcher.py 
> ae1b24bf4e3291cb31b3129cabcacdf32db0c560 
>   src/test/python/apache/aurora/client/api/test_sla.py 
> 1117f24d5ad3640632a1dd728913ba73c8bec707 
>   src/test/python/apache/aurora/client/cli/test_api_from_cli.py 
> 95aa649cfff9166dd10aa432c4d470739e8f06c5 
>   src/test/python/apache/aurora/client/cli/test_diff.py 
> 10817695352687cdb5b0c3ed9720e3091b230e68 
>   src/test/python/apache/aurora/client/cli/test_status.py 
> 4f62cf0c52e5837309cf7ad702df6d907df8f510 
>   src/test/python/apache/aurora/client/cli/test_task_run.py 
> 16fde14c03f6fd2c000e76625fad174835763f1b 
>   src/test/python/apache/aurora/client/cli/test_update.py 
> 1ec5483506a22a774340acccd33f09f1742be8b7 
>   src/test/python/apache/aurora/client/cli/util.py 
> 3fa609a5f71525393ca0a5dbd81423005fadb583 
>   src/test/python/apache/aurora/client/commands/test_diff.py 
> c8d01456aa52fd61374b4f0960b5159da2cb235b 
>   src/test/python/apache/aurora/client/commands/test_ssh.py 
> abb657ba397c23ddac6c6b188f70d1c4e34597a6 
>   src/test/python/apache/aurora/client/commands/test_status.py 
> 639763501348a35bff2f127e18780ac74852f51b 
>   src/test/python/apache/aurora/client/commands/test_update.py 
> 07cbe53109e8bcdd09dcac47f6353b10e095717d 
>   src/test/python/apache/aurora/config/test_thrift.py 
> 1dd9e79099793aa7bb377c20c2a6a0c4b750be33 
>   src/test/python/apache/aurora/executor/common/test_announcer.py 
> 56943351ca09c29580dd764bb2442f0fcd9fde74 
>   src/test/python/apache/aurora/executor/test_thermos_executor.py 
> 65e8cce60a5543081175c574aaaf92f200bc6233 
> 
> Diff: https://reviews.apache.org/r/26954/diff/
> 
> 
> Testing
> ---
> 
> ./pants src/test/python:all
> test_end_to_end.sh
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 26954: Preparing for Identity struct deprecation (client and executor).

[Bill Farner]

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


In case you skimmed past it - the Mock thing would be great to address here.

- Bill Farner


On Oct. 22, 2014, 11:59 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26954/
> ---
> 
> (Updated Oct. 22, 2014, 11:59 p.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Bill Farner.
> 
> 
> Bugs: AURORA-84
> https://issues.apache.org/jira/browse/AURORA-84
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Python side of changes for deprecating identity struct. Accounting for the 
> upcoming deprecation by backfilling missing fields and switching between 
> old/new fields (whichever is available).
> 
> Branched off of https://reviews.apache.org/r/26762/ and will have to be 
> pushed together.
> 
> Summary of changes:
> - TaskConfig - backfilling _key_ field in job diff and update commands.
> 
> Also, some unit test refactoring to avoid copy-pasted fragments.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/client/api/instance_watcher.py 
> b390aa8993205f1f6938f8c295e3c16a0bf4df6d 
>   src/main/python/apache/aurora/client/api/sla.py 
> b9b64680b15f5395ed6aca681b9b1c30ffe2822c 
>   src/main/python/apache/aurora/client/cli/task.py 
> c41484bdc27266443bc4e139e1ebb362a59be0f9 
>   src/main/python/apache/aurora/client/commands/admin.py 
> deee0250f3ba9837feeb92acc654f5b3b68b4e0f 
>   src/main/python/apache/aurora/client/commands/core.py 
> 58f419e674f1a9a0ae9da6faa2e39c8167bab597 
>   src/main/python/apache/aurora/client/commands/ssh.py 
> d2b8bf675556b924d3d63b545d036dc48a081486 
>   src/main/python/apache/aurora/config/thrift.py 
> 9ca806db6d2c59836196ea2fdd3fc015b9f6b71f 
>   src/main/python/apache/aurora/executor/aurora_executor.py 
> 2c6423d096656f426a4385f4edef6875ebad7049 
>   src/main/python/apache/aurora/executor/common/announcer.py 
> 74b2114d1ede7a4a4a68b78144f9b08bc994bd87 
>   src/main/python/apache/aurora/executor/thermos_task_runner.py 
> bb99bd12d7387e080d8d4c0557d2afa3bf5e7c60 
>   src/test/python/apache/aurora/client/api/test_instance_watcher.py 
> ae1b24bf4e3291cb31b3129cabcacdf32db0c560 
>   src/test/python/apache/aurora/client/api/test_sla.py 
> 1117f24d5ad3640632a1dd728913ba73c8bec707 
>   src/test/python/apache/aurora/client/cli/test_api_from_cli.py 
> 95aa649cfff9166dd10aa432c4d470739e8f06c5 
>   src/test/python/apache/aurora/client/cli/test_diff.py 
> 10817695352687cdb5b0c3ed9720e3091b230e68 
>   src/test/python/apache/aurora/client/cli/test_status.py 
> 4f62cf0c52e5837309cf7ad702df6d907df8f510 
>   src/test/python/apache/aurora/client/cli/test_task_run.py 
> 16fde14c03f6fd2c000e76625fad174835763f1b 
>   src/test/python/apache/aurora/client/cli/test_update.py 
> 1ec5483506a22a774340acccd33f09f1742be8b7 
>   src/test/python/apache/aurora/client/cli/util.py 
> 3fa609a5f71525393ca0a5dbd81423005fadb583 
>   src/test/python/apache/aurora/client/commands/test_diff.py 
> c8d01456aa52fd61374b4f0960b5159da2cb235b 
>   src/test/python/apache/aurora/client/commands/test_ssh.py 
> abb657ba397c23ddac6c6b188f70d1c4e34597a6 
>   src/test/python/apache/aurora/client/commands/test_status.py 
> 639763501348a35bff2f127e18780ac74852f51b 
>   src/test/python/apache/aurora/client/commands/test_update.py 
> 07cbe53109e8bcdd09dcac47f6353b10e095717d 
>   src/test/python/apache/aurora/config/test_thrift.py 
> 1dd9e79099793aa7bb377c20c2a6a0c4b750be33 
>   src/test/python/apache/aurora/executor/common/test_announcer.py 
> 56943351ca09c29580dd764bb2442f0fcd9fde74 
>   src/test/python/apache/aurora/executor/test_thermos_executor.py 
> 65e8cce60a5543081175c574aaaf92f200bc6233 
> 
> Diff: https://reviews.apache.org/r/26954/diff/
> 
> 
> Testing
> ---
> 
> ./pants src/test/python:all
> test_end_to_end.sh
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 26954: Preparing for Identity struct deprecation (client and executor).

[Maxim Khutornenko]

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

(Updated Oct. 22, 2014, 11:59 p.m.)


Review request for Aurora, Kevin Sweeney and Bill Farner.


Changes
---

CR comments.


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


Repository: aurora


Description
---

Python side of changes for deprecating identity struct. Accounting for the 
upcoming deprecation by backfilling missing fields and switching between 
old/new fields (whichever is available).

Branched off of https://reviews.apache.org/r/26762/ and will have to be pushed 
together.

Summary of changes:
- TaskConfig - backfilling _key_ field in job diff and update commands.

Also, some unit test refactoring to avoid copy-pasted fragments.


Diffs (updated)
-

  src/main/python/apache/aurora/client/api/instance_watcher.py 
b390aa8993205f1f6938f8c295e3c16a0bf4df6d 
  src/main/python/apache/aurora/client/api/sla.py 
b9b64680b15f5395ed6aca681b9b1c30ffe2822c 
  src/main/python/apache/aurora/client/cli/task.py 
c41484bdc27266443bc4e139e1ebb362a59be0f9 
  src/main/python/apache/aurora/client/commands/admin.py 
deee0250f3ba9837feeb92acc654f5b3b68b4e0f 
  src/main/python/apache/aurora/client/commands/core.py 
58f419e674f1a9a0ae9da6faa2e39c8167bab597 
  src/main/python/apache/aurora/client/commands/ssh.py 
d2b8bf675556b924d3d63b545d036dc48a081486 
  src/main/python/apache/aurora/config/thrift.py 
9ca806db6d2c59836196ea2fdd3fc015b9f6b71f 
  src/main/python/apache/aurora/executor/aurora_executor.py 
2c6423d096656f426a4385f4edef6875ebad7049 
  src/main/python/apache/aurora/executor/common/announcer.py 
74b2114d1ede7a4a4a68b78144f9b08bc994bd87 
  src/main/python/apache/aurora/executor/thermos_task_runner.py 
bb99bd12d7387e080d8d4c0557d2afa3bf5e7c60 
  src/test/python/apache/aurora/client/api/test_instance_watcher.py 
ae1b24bf4e3291cb31b3129cabcacdf32db0c560 
  src/test/python/apache/aurora/client/api/test_sla.py 
1117f24d5ad3640632a1dd728913ba73c8bec707 
  src/test/python/apache/aurora/client/cli/test_api_from_cli.py 
95aa649cfff9166dd10aa432c4d470739e8f06c5 
  src/test/python/apache/aurora/client/cli/test_diff.py 
10817695352687cdb5b0c3ed9720e3091b230e68 
  src/test/python/apache/aurora/client/cli/test_status.py 
4f62cf0c52e5837309cf7ad702df6d907df8f510 
  src/test/python/apache/aurora/client/cli/test_task_run.py 
16fde14c03f6fd2c000e76625fad174835763f1b 
  src/test/python/apache/aurora/client/cli/test_update.py 
1ec5483506a22a774340acccd33f09f1742be8b7 
  src/test/python/apache/aurora/client/cli/util.py 
3fa609a5f71525393ca0a5dbd81423005fadb583 
  src/test/python/apache/aurora/client/commands/test_diff.py 
c8d01456aa52fd61374b4f0960b5159da2cb235b 
  src/test/python/apache/aurora/client/commands/test_ssh.py 
abb657ba397c23ddac6c6b188f70d1c4e34597a6 
  src/test/python/apache/aurora/client/commands/test_status.py 
639763501348a35bff2f127e18780ac74852f51b 
  src/test/python/apache/aurora/client/commands/test_update.py 
07cbe53109e8bcdd09dcac47f6353b10e095717d 
  src/test/python/apache/aurora/config/test_thrift.py 
1dd9e79099793aa7bb377c20c2a6a0c4b750be33 
  src/test/python/apache/aurora/executor/common/test_announcer.py 
56943351ca09c29580dd764bb2442f0fcd9fde74 
  src/test/python/apache/aurora/executor/test_thermos_executor.py 
65e8cce60a5543081175c574aaaf92f200bc6233 

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


Testing
---

./pants src/test/python:all
test_end_to_end.sh


Thanks,

Maxim Khutornenko

Re: Review Request 26954: Preparing for Identity struct deprecation (client and executor).

[Bill Farner]

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


LG overall, aside for what seems to be unnecessary field modification when 
diffing configs.


src/main/python/apache/aurora/client/api/updater.py


Echoing our offline discussion - this is unnecessary, right?  Since 
`from_config` and `to_config` are both from the scheduler, we can assume it is 
internally consistent?



src/main/python/apache/aurora/client/cli/jobs.py


Ditto - should be unnecessary IIUC.



src/test/python/apache/aurora/client/api/test_updater.py


Ditto - this should be unnecessary, since both configs are round-tripped 
through the scheduler.



src/test/python/apache/aurora/client/cli/test_diff.py


While you're here, please change this to not use Mock, all down the 
hierarchy.

Ditto for other cargo cults (thanks for collapsing a few, btw).


- Bill Farner


On Oct. 22, 2014, 11:11 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26954/
> ---
> 
> (Updated Oct. 22, 2014, 11:11 p.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Bill Farner.
> 
> 
> Bugs: AURORA-84
> https://issues.apache.org/jira/browse/AURORA-84
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Python side of changes for deprecating identity struct. Accounting for the 
> upcoming deprecation by backfilling missing fields and switching between 
> old/new fields (whichever is available).
> 
> Branched off of https://reviews.apache.org/r/26762/ and will have to be 
> pushed together.
> 
> Summary of changes:
> - TaskConfig - backfilling _key_ field in job diff and update commands.
> 
> Also, some unit test refactoring to avoid copy-pasted fragments.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/client/api/instance_watcher.py 
> b390aa8993205f1f6938f8c295e3c16a0bf4df6d 
>   src/main/python/apache/aurora/client/api/sla.py 
> b9b64680b15f5395ed6aca681b9b1c30ffe2822c 
>   src/main/python/apache/aurora/client/api/updater.py 
> 2e6bc9fdc1c745b2a69f8929209d46517c218700 
>   src/main/python/apache/aurora/client/cli/jobs.py 
> 10f8e0d331ca607e55e3aa6f96014caea744ed9f 
>   src/main/python/apache/aurora/client/cli/task.py 
> c41484bdc27266443bc4e139e1ebb362a59be0f9 
>   src/main/python/apache/aurora/client/commands/admin.py 
> deee0250f3ba9837feeb92acc654f5b3b68b4e0f 
>   src/main/python/apache/aurora/client/commands/core.py 
> 58f419e674f1a9a0ae9da6faa2e39c8167bab597 
>   src/main/python/apache/aurora/client/commands/ssh.py 
> d2b8bf675556b924d3d63b545d036dc48a081486 
>   src/main/python/apache/aurora/config/thrift.py 
> 9ca806db6d2c59836196ea2fdd3fc015b9f6b71f 
>   src/main/python/apache/aurora/executor/aurora_executor.py 
> 2c6423d096656f426a4385f4edef6875ebad7049 
>   src/main/python/apache/aurora/executor/common/announcer.py 
> 74b2114d1ede7a4a4a68b78144f9b08bc994bd87 
>   src/main/python/apache/aurora/executor/thermos_task_runner.py 
> bb99bd12d7387e080d8d4c0557d2afa3bf5e7c60 
>   src/test/python/apache/aurora/client/api/test_instance_watcher.py 
> ae1b24bf4e3291cb31b3129cabcacdf32db0c560 
>   src/test/python/apache/aurora/client/api/test_sla.py 
> 1117f24d5ad3640632a1dd728913ba73c8bec707 
>   src/test/python/apache/aurora/client/api/test_updater.py 
> f0e45e3f33de0dcb87251021bbf96bfea6f38ea8 
>   src/test/python/apache/aurora/client/cli/test_api_from_cli.py 
> 95aa649cfff9166dd10aa432c4d470739e8f06c5 
>   src/test/python/apache/aurora/client/cli/test_diff.py 
> 10817695352687cdb5b0c3ed9720e3091b230e68 
>   src/test/python/apache/aurora/client/cli/test_status.py 
> 4f62cf0c52e5837309cf7ad702df6d907df8f510 
>   src/test/python/apache/aurora/client/cli/test_task_run.py 
> 16fde14c03f6fd2c000e76625fad174835763f1b 
>   src/test/python/apache/aurora/client/cli/test_update.py 
> 1ec5483506a22a774340acccd33f09f1742be8b7 
>   src/test/python/apache/aurora/client/cli/util.py 
> 3fa609a5f71525393ca0a5dbd81423005fadb583 
>   src/test/python/apache/aurora/client/commands/test_diff.py 
> c8d01456aa52fd61374b4f0960b5159da2cb235b 
>   src/test/python/apache/aurora/client/commands/test_ssh.py 
> abb657ba397c23ddac6c6b188f70d1c4e34597a6 
>   src/test/python/apache/aurora/client/commands/test_status.py 
> 639763501348a35bff2f127e18780ac74852f51b 
>   src/test/python/apache/aurora/client/commands/test_update.py 
> 07cbe53109e8bcdd09dcac47f6353b10e095717d 
>   src/test/python/apache/aurora/config/test_thrift.py 
> 1dd9e79099793aa7bb377c20c2a6a0c4b750be33 
>   src/test/p

Re: Review Request 26954: Preparing for Identity struct deprecation (client and executor).

[Maxim Khutornenko]

> On Oct. 22, 2014, 9:58 p.m., Bill Farner wrote:
> > Is it worth the risk to update the client code now?  Seems like we can ride 
> > it out until the field is removed with lower overall risk.

If not done now the client diff will be immediately broken as a new 
TaskConfig.key field will never be populated on the client but will always get 
returned by the scheduler.

Also, without a `config.key.role if config.key else config.owner.role` switch 
the rest of the client will break in a future release when scheduler stops 
returning deprecated fields.


- Maxim


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


On Oct. 22, 2014, 9:10 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26954/
> ---
> 
> (Updated Oct. 22, 2014, 9:10 p.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Bill Farner.
> 
> 
> Bugs: AURORA-84
> https://issues.apache.org/jira/browse/AURORA-84
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Python side of changes for deprecating identity struct. Accounting for the 
> upcoming deprecation by backfilling missing fields and switching between 
> old/new fields (whichever is available).
> 
> Branched off of https://reviews.apache.org/r/26762/ and will have to be 
> pushed together.
> 
> Summary of changes:
> - TaskConfig - backfilling _key_ field in job diff and update commands.
> 
> Also, some unit test refactoring to avoid copy-pasted fragments.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/client/api/instance_watcher.py 
> b390aa8993205f1f6938f8c295e3c16a0bf4df6d 
>   src/main/python/apache/aurora/client/api/sla.py 
> b9b64680b15f5395ed6aca681b9b1c30ffe2822c 
>   src/main/python/apache/aurora/client/api/updater.py 
> 2e6bc9fdc1c745b2a69f8929209d46517c218700 
>   src/main/python/apache/aurora/client/cli/jobs.py 
> 0277cbed4b7eb927d6e5823b4b41d90b366f81d0 
>   src/main/python/apache/aurora/client/cli/task.py 
> c41484bdc27266443bc4e139e1ebb362a59be0f9 
>   src/main/python/apache/aurora/client/commands/admin.py 
> deee0250f3ba9837feeb92acc654f5b3b68b4e0f 
>   src/main/python/apache/aurora/client/commands/core.py 
> 58f419e674f1a9a0ae9da6faa2e39c8167bab597 
>   src/main/python/apache/aurora/client/commands/ssh.py 
> d2b8bf675556b924d3d63b545d036dc48a081486 
>   src/main/python/apache/aurora/config/thrift.py 
> 9ca806db6d2c59836196ea2fdd3fc015b9f6b71f 
>   src/main/python/apache/aurora/executor/aurora_executor.py 
> 2c6423d096656f426a4385f4edef6875ebad7049 
>   src/main/python/apache/aurora/executor/common/announcer.py 
> 74b2114d1ede7a4a4a68b78144f9b08bc994bd87 
>   src/main/python/apache/aurora/executor/thermos_task_runner.py 
> f17910826c31c5bf754b43eb72500de639652f37 
>   src/test/python/apache/aurora/client/api/test_instance_watcher.py 
> ae1b24bf4e3291cb31b3129cabcacdf32db0c560 
>   src/test/python/apache/aurora/client/api/test_sla.py 
> 1117f24d5ad3640632a1dd728913ba73c8bec707 
>   src/test/python/apache/aurora/client/api/test_updater.py 
> f0e45e3f33de0dcb87251021bbf96bfea6f38ea8 
>   src/test/python/apache/aurora/client/cli/test_api_from_cli.py 
> 95aa649cfff9166dd10aa432c4d470739e8f06c5 
>   src/test/python/apache/aurora/client/cli/test_diff.py 
> 10817695352687cdb5b0c3ed9720e3091b230e68 
>   src/test/python/apache/aurora/client/cli/test_status.py 
> c704daec5a6eee73c7092a201b168881853908e8 
>   src/test/python/apache/aurora/client/cli/test_task_run.py 
> 16fde14c03f6fd2c000e76625fad174835763f1b 
>   src/test/python/apache/aurora/client/cli/test_update.py 
> cff1b6578aec6f5bcc1e610e58b47af233f32b41 
>   src/test/python/apache/aurora/client/cli/util.py 
> 3fa609a5f71525393ca0a5dbd81423005fadb583 
>   src/test/python/apache/aurora/client/commands/test_diff.py 
> c8d01456aa52fd61374b4f0960b5159da2cb235b 
>   src/test/python/apache/aurora/client/commands/test_ssh.py 
> abb657ba397c23ddac6c6b188f70d1c4e34597a6 
>   src/test/python/apache/aurora/client/commands/test_status.py 
> 639763501348a35bff2f127e18780ac74852f51b 
>   src/test/python/apache/aurora/client/commands/test_update.py 
> 07cbe53109e8bcdd09dcac47f6353b10e095717d 
>   src/test/python/apache/aurora/config/test_thrift.py 
> 1dd9e79099793aa7bb377c20c2a6a0c4b750be33 
>   src/test/python/apache/aurora/executor/common/test_announcer.py 
> 56943351ca09c29580dd764bb2442f0fcd9fde74 
>   src/test/python/apache/aurora/executor/test_thermos_executor.py 
> 65e8cce60a5543081175c574aaaf92f200bc6233 
> 
> Diff: https://reviews.apache.org/r/26954/diff/
> 
> 
> Testing
> ---
> 
> ./pants src/test/python:all
> test_end_to_end.sh
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 26954: Preparing for Identity struct deprecation (client and executor).

[Bill Farner]

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


Is it worth the risk to update the client code now?  Seems like we can ride it 
out until the field is removed with lower overall risk.

- Bill Farner


On Oct. 22, 2014, 9:10 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26954/
> ---
> 
> (Updated Oct. 22, 2014, 9:10 p.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Bill Farner.
> 
> 
> Bugs: AURORA-84
> https://issues.apache.org/jira/browse/AURORA-84
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Python side of changes for deprecating identity struct. Accounting for the 
> upcoming deprecation by backfilling missing fields and switching between 
> old/new fields (whichever is available).
> 
> Branched off of https://reviews.apache.org/r/26762/ and will have to be 
> pushed together.
> 
> Summary of changes:
> - TaskConfig - backfilling _key_ field in job diff and update commands.
> 
> Also, some unit test refactoring to avoid copy-pasted fragments.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/client/api/instance_watcher.py 
> b390aa8993205f1f6938f8c295e3c16a0bf4df6d 
>   src/main/python/apache/aurora/client/api/sla.py 
> b9b64680b15f5395ed6aca681b9b1c30ffe2822c 
>   src/main/python/apache/aurora/client/api/updater.py 
> 2e6bc9fdc1c745b2a69f8929209d46517c218700 
>   src/main/python/apache/aurora/client/cli/jobs.py 
> 0277cbed4b7eb927d6e5823b4b41d90b366f81d0 
>   src/main/python/apache/aurora/client/cli/task.py 
> c41484bdc27266443bc4e139e1ebb362a59be0f9 
>   src/main/python/apache/aurora/client/commands/admin.py 
> deee0250f3ba9837feeb92acc654f5b3b68b4e0f 
>   src/main/python/apache/aurora/client/commands/core.py 
> 58f419e674f1a9a0ae9da6faa2e39c8167bab597 
>   src/main/python/apache/aurora/client/commands/ssh.py 
> d2b8bf675556b924d3d63b545d036dc48a081486 
>   src/main/python/apache/aurora/config/thrift.py 
> 9ca806db6d2c59836196ea2fdd3fc015b9f6b71f 
>   src/main/python/apache/aurora/executor/aurora_executor.py 
> 2c6423d096656f426a4385f4edef6875ebad7049 
>   src/main/python/apache/aurora/executor/common/announcer.py 
> 74b2114d1ede7a4a4a68b78144f9b08bc994bd87 
>   src/main/python/apache/aurora/executor/thermos_task_runner.py 
> f17910826c31c5bf754b43eb72500de639652f37 
>   src/test/python/apache/aurora/client/api/test_instance_watcher.py 
> ae1b24bf4e3291cb31b3129cabcacdf32db0c560 
>   src/test/python/apache/aurora/client/api/test_sla.py 
> 1117f24d5ad3640632a1dd728913ba73c8bec707 
>   src/test/python/apache/aurora/client/api/test_updater.py 
> f0e45e3f33de0dcb87251021bbf96bfea6f38ea8 
>   src/test/python/apache/aurora/client/cli/test_api_from_cli.py 
> 95aa649cfff9166dd10aa432c4d470739e8f06c5 
>   src/test/python/apache/aurora/client/cli/test_diff.py 
> 10817695352687cdb5b0c3ed9720e3091b230e68 
>   src/test/python/apache/aurora/client/cli/test_status.py 
> c704daec5a6eee73c7092a201b168881853908e8 
>   src/test/python/apache/aurora/client/cli/test_task_run.py 
> 16fde14c03f6fd2c000e76625fad174835763f1b 
>   src/test/python/apache/aurora/client/cli/test_update.py 
> cff1b6578aec6f5bcc1e610e58b47af233f32b41 
>   src/test/python/apache/aurora/client/cli/util.py 
> 3fa609a5f71525393ca0a5dbd81423005fadb583 
>   src/test/python/apache/aurora/client/commands/test_diff.py 
> c8d01456aa52fd61374b4f0960b5159da2cb235b 
>   src/test/python/apache/aurora/client/commands/test_ssh.py 
> abb657ba397c23ddac6c6b188f70d1c4e34597a6 
>   src/test/python/apache/aurora/client/commands/test_status.py 
> 639763501348a35bff2f127e18780ac74852f51b 
>   src/test/python/apache/aurora/client/commands/test_update.py 
> 07cbe53109e8bcdd09dcac47f6353b10e095717d 
>   src/test/python/apache/aurora/config/test_thrift.py 
> 1dd9e79099793aa7bb377c20c2a6a0c4b750be33 
>   src/test/python/apache/aurora/executor/common/test_announcer.py 
> 56943351ca09c29580dd764bb2442f0fcd9fde74 
>   src/test/python/apache/aurora/executor/test_thermos_executor.py 
> 65e8cce60a5543081175c574aaaf92f200bc6233 
> 
> Diff: https://reviews.apache.org/r/26954/diff/
> 
> 
> Testing
> ---
> 
> ./pants src/test/python:all
> test_end_to_end.sh
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 26954: Preparing for Identity struct deprecation (client and executor).

[Maxim Khutornenko]

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

(Updated Oct. 22, 2014, 9:10 p.m.)


Review request for Aurora, Kevin Sweeney and Bill Farner.


Changes
---

Updating title and description.


Summary (updated)
-

Preparing for Identity struct deprecation (client and executor).


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


Repository: aurora


Description (updated)
---

Python side of changes for deprecating identity struct. Accounting for the 
upcoming deprecation by backfilling missing fields and switching between 
old/new fields (whichever is available).

Branched off of https://reviews.apache.org/r/26762/ and will have to be pushed 
together.

Summary of changes:
- TaskConfig - backfilling _key_ field in job diff and update commands.

Also, some unit test refactoring to avoid copy-pasted fragments.


Diffs
-

  src/main/python/apache/aurora/client/api/instance_watcher.py 
b390aa8993205f1f6938f8c295e3c16a0bf4df6d 
  src/main/python/apache/aurora/client/api/sla.py 
b9b64680b15f5395ed6aca681b9b1c30ffe2822c 
  src/main/python/apache/aurora/client/api/updater.py 
2e6bc9fdc1c745b2a69f8929209d46517c218700 
  src/main/python/apache/aurora/client/cli/jobs.py 
0277cbed4b7eb927d6e5823b4b41d90b366f81d0 
  src/main/python/apache/aurora/client/cli/task.py 
c41484bdc27266443bc4e139e1ebb362a59be0f9 
  src/main/python/apache/aurora/client/commands/admin.py 
deee0250f3ba9837feeb92acc654f5b3b68b4e0f 
  src/main/python/apache/aurora/client/commands/core.py 
58f419e674f1a9a0ae9da6faa2e39c8167bab597 
  src/main/python/apache/aurora/client/commands/ssh.py 
d2b8bf675556b924d3d63b545d036dc48a081486 
  src/main/python/apache/aurora/config/thrift.py 
9ca806db6d2c59836196ea2fdd3fc015b9f6b71f 
  src/main/python/apache/aurora/executor/aurora_executor.py 
2c6423d096656f426a4385f4edef6875ebad7049 
  src/main/python/apache/aurora/executor/common/announcer.py 
74b2114d1ede7a4a4a68b78144f9b08bc994bd87 
  src/main/python/apache/aurora/executor/thermos_task_runner.py 
f17910826c31c5bf754b43eb72500de639652f37 
  src/test/python/apache/aurora/client/api/test_instance_watcher.py 
ae1b24bf4e3291cb31b3129cabcacdf32db0c560 
  src/test/python/apache/aurora/client/api/test_sla.py 
1117f24d5ad3640632a1dd728913ba73c8bec707 
  src/test/python/apache/aurora/client/api/test_updater.py 
f0e45e3f33de0dcb87251021bbf96bfea6f38ea8 
  src/test/python/apache/aurora/client/cli/test_api_from_cli.py 
95aa649cfff9166dd10aa432c4d470739e8f06c5 
  src/test/python/apache/aurora/client/cli/test_diff.py 
10817695352687cdb5b0c3ed9720e3091b230e68 
  src/test/python/apache/aurora/client/cli/test_status.py 
c704daec5a6eee73c7092a201b168881853908e8 
  src/test/python/apache/aurora/client/cli/test_task_run.py 
16fde14c03f6fd2c000e76625fad174835763f1b 
  src/test/python/apache/aurora/client/cli/test_update.py 
cff1b6578aec6f5bcc1e610e58b47af233f32b41 
  src/test/python/apache/aurora/client/cli/util.py 
3fa609a5f71525393ca0a5dbd81423005fadb583 
  src/test/python/apache/aurora/client/commands/test_diff.py 
c8d01456aa52fd61374b4f0960b5159da2cb235b 
  src/test/python/apache/aurora/client/commands/test_ssh.py 
abb657ba397c23ddac6c6b188f70d1c4e34597a6 
  src/test/python/apache/aurora/client/commands/test_status.py 
639763501348a35bff2f127e18780ac74852f51b 
  src/test/python/apache/aurora/client/commands/test_update.py 
07cbe53109e8bcdd09dcac47f6353b10e095717d 
  src/test/python/apache/aurora/config/test_thrift.py 
1dd9e79099793aa7bb377c20c2a6a0c4b750be33 
  src/test/python/apache/aurora/executor/common/test_announcer.py 
56943351ca09c29580dd764bb2442f0fcd9fde74 
  src/test/python/apache/aurora/executor/test_thermos_executor.py 
65e8cce60a5543081175c574aaaf92f200bc6233 

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


Testing
---

./pants src/test/python:all
test_end_to_end.sh


Thanks,

Maxim Khutornenko