Re: Review Request 39532: Always set SessionKey to empty in the client.

2015-10-22 Thread Kevin Sweeney

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


@reviewbot retry

- Kevin Sweeney


On Oct. 22, 2015, 10:36 a.m., Kevin Sweeney wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39532/
> ---
> 
> (Updated Oct. 22, 2015, 10:36 a.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Always set SessionKey to empty on the client, as it's now ignored by the 
> scheduler.
> 
> 
> Diffs
> -
> 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> f0d4ef824562093492a8f3c9efa2059908f4d98b 
>   src/main/python/apache/aurora/client/api/__init__.py 
> 5847ca88b0aeb828e7d03538725b3430ecd209ab 
>   src/main/python/apache/aurora/client/api/scheduler_client.py 
> 8e91788d8cb69ef21df6b045cd07f8cb111b95b3 
>   src/main/python/apache/aurora/common/auth/auth_module.py 
> 53a3182896a9d385899e1f0274b2bfbe053076bb 
>   src/main/python/apache/aurora/common/auth/auth_module_manager.py 
> 73c6bd76989d97e4e6c336eb2fd9970b4c5e5b5c 
>   src/test/python/apache/aurora/client/api/test_restarter.py 
> 3b14d888b52241927a1005a518516174e907d7eb 
>   src/test/python/apache/aurora/client/api/test_scheduler_client.py 
> 10e8ebb4a12cc39d296cfde64ed9b01119d0aac3 
>   src/test/python/apache/aurora/client/api/test_updater.py 
> 87935553d37db8f0a1d03d3c370cf717b5277d74 
> 
> Diff: https://reviews.apache.org/r/39532/diff/
> 
> 
> Testing
> ---
> 
> ./pants test.pytest --no-fast src/test/python/:: 
> 
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Kevin Sweeney
> 
>



Re: Review Request 39532: Always set SessionKey to empty in the client.

2015-10-22 Thread Kevin Sweeney

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

(Updated Oct. 22, 2015, 10:36 a.m.)


Review request for Aurora, Maxim Khutornenko and Zameer Manji.


Changes
---

Also remove fields and document deprecation in api.thrift.


Repository: aurora


Description
---

Always set SessionKey to empty on the client, as it's now ignored by the 
scheduler.


Diffs (updated)
-

  api/src/main/thrift/org/apache/aurora/gen/api.thrift 
f0d4ef824562093492a8f3c9efa2059908f4d98b 
  src/main/python/apache/aurora/client/api/__init__.py 
5847ca88b0aeb828e7d03538725b3430ecd209ab 
  src/main/python/apache/aurora/client/api/scheduler_client.py 
8e91788d8cb69ef21df6b045cd07f8cb111b95b3 
  src/main/python/apache/aurora/common/auth/auth_module.py 
53a3182896a9d385899e1f0274b2bfbe053076bb 
  src/main/python/apache/aurora/common/auth/auth_module_manager.py 
73c6bd76989d97e4e6c336eb2fd9970b4c5e5b5c 
  src/test/python/apache/aurora/client/api/test_restarter.py 
3b14d888b52241927a1005a518516174e907d7eb 
  src/test/python/apache/aurora/client/api/test_scheduler_client.py 
10e8ebb4a12cc39d296cfde64ed9b01119d0aac3 
  src/test/python/apache/aurora/client/api/test_updater.py 
87935553d37db8f0a1d03d3c370cf717b5277d74 

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


Testing
---

./pants test.pytest --no-fast src/test/python/:: 

./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh


Thanks,

Kevin Sweeney



Re: Review Request 39532: Always set SessionKey to empty in the client.

2015-10-22 Thread Kevin Sweeney


> On Oct. 22, 2015, 10:42 a.m., Maxim Khutornenko wrote:
> > api/src/main/thrift/org/apache/aurora/gen/api.thrift, lines 56-62
> > 
> >
> > Why removing these fields now? This does not feel like a safe 
> > deprecation approach.

v0.9.0 schedulers ignore these fields, they just happen to call requireNonNull 
on this object. In thrift it is always fine to send extra fields that the 
deserializer is unaware of; they will simply be dropped on the floor.


- Kevin


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


On Oct. 22, 2015, 10:36 a.m., Kevin Sweeney wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39532/
> ---
> 
> (Updated Oct. 22, 2015, 10:36 a.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Always set SessionKey to empty on the client, as it's now ignored by the 
> scheduler.
> 
> 
> Diffs
> -
> 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> f0d4ef824562093492a8f3c9efa2059908f4d98b 
>   src/main/python/apache/aurora/client/api/__init__.py 
> 5847ca88b0aeb828e7d03538725b3430ecd209ab 
>   src/main/python/apache/aurora/client/api/scheduler_client.py 
> 8e91788d8cb69ef21df6b045cd07f8cb111b95b3 
>   src/main/python/apache/aurora/common/auth/auth_module.py 
> 53a3182896a9d385899e1f0274b2bfbe053076bb 
>   src/main/python/apache/aurora/common/auth/auth_module_manager.py 
> 73c6bd76989d97e4e6c336eb2fd9970b4c5e5b5c 
>   src/test/python/apache/aurora/client/api/test_restarter.py 
> 3b14d888b52241927a1005a518516174e907d7eb 
>   src/test/python/apache/aurora/client/api/test_scheduler_client.py 
> 10e8ebb4a12cc39d296cfde64ed9b01119d0aac3 
>   src/test/python/apache/aurora/client/api/test_updater.py 
> 87935553d37db8f0a1d03d3c370cf717b5277d74 
> 
> Diff: https://reviews.apache.org/r/39532/diff/
> 
> 
> Testing
> ---
> 
> ./pants test.pytest --no-fast src/test/python/:: 
> 
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Kevin Sweeney
> 
>



Re: Review Request 39532: Always set SessionKey to empty in the client.

2015-10-22 Thread Aurora ReviewBot

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

Ship it!


Master (4eeec7a) 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 Oct. 22, 2015, 5:36 p.m., Kevin Sweeney wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39532/
> ---
> 
> (Updated Oct. 22, 2015, 5:36 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Always set SessionKey to empty on the client, as it's now ignored by the 
> scheduler.
> 
> 
> Diffs
> -
> 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> f0d4ef824562093492a8f3c9efa2059908f4d98b 
>   src/main/python/apache/aurora/client/api/__init__.py 
> 5847ca88b0aeb828e7d03538725b3430ecd209ab 
>   src/main/python/apache/aurora/client/api/scheduler_client.py 
> 8e91788d8cb69ef21df6b045cd07f8cb111b95b3 
>   src/main/python/apache/aurora/common/auth/auth_module.py 
> 53a3182896a9d385899e1f0274b2bfbe053076bb 
>   src/main/python/apache/aurora/common/auth/auth_module_manager.py 
> 73c6bd76989d97e4e6c336eb2fd9970b4c5e5b5c 
>   src/test/python/apache/aurora/client/api/test_restarter.py 
> 3b14d888b52241927a1005a518516174e907d7eb 
>   src/test/python/apache/aurora/client/api/test_scheduler_client.py 
> 10e8ebb4a12cc39d296cfde64ed9b01119d0aac3 
>   src/test/python/apache/aurora/client/api/test_updater.py 
> 87935553d37db8f0a1d03d3c370cf717b5277d74 
> 
> Diff: https://reviews.apache.org/r/39532/diff/
> 
> 
> Testing
> ---
> 
> ./pants test.pytest --no-fast src/test/python/:: 
> 
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Kevin Sweeney
> 
>



Re: Review Request 39532: Always set SessionKey to empty in the client.

2015-10-22 Thread Aurora ReviewBot

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


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

 src.test.python.apache.aurora.client.cli.plugins   
 .   SUCCESS
 src.test.python.apache.aurora.client.cli.quota 
 .   SUCCESS
 src.test.python.apache.aurora.client.cli.sla   
 .   SUCCESS
 src.test.python.apache.aurora.client.cli.supdate   
 .   SUCCESS
 src.test.python.apache.aurora.client.cli.task  
 .   SUCCESS
 src.test.python.apache.aurora.client.cli.update
 .   SUCCESS
 src.test.python.apache.aurora.client.cli.version   
 .   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_pex_version  
 .   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.common.path_detector 
.   SUCCESS
 src.test.python.apache.aurora.executor.common.task_info
 .   SUCCESS
 src.test.python.apache.aurora.executor.executor_base   
 .   SUCCESS
 src.test.python.apache.aurora.executor.executor_vars   
 .   SUCCESS
 src.test.python.apache.aurora.executor.http_lifecycle  
 .   SUCCESS
 src.test.python.apache.aurora.executor.status_manager  
 .   SUCCESS
 src.test.python.apache.aurora.executor.thermos_task_runner 
 .   FAILURE
 src.test.python.apache.thermos.cli.commands.commands   
 .   SUCCESS
 src.test.python.apache.thermos.cli.common  
 .   SUCCESS
 src.test.python.apache.thermos.cli.main
 .   SUCCESS
 src.test.python.apache.thermos.common.test_pathspec
 .   SUCCESS
 
src.test.python.apache.thermos.core.test_runner_integration 
.   SUCCESS
 src.test.python.apache.thermos.monitoring.test_disk
 .   SUCCESS
 
FAILURE


   FAILURE


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

- Aurora ReviewBot


On Oct. 22, 2015, 5:36 p.m., Kevin Sweeney wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39532/
> ---
> 
> (Updated Oct. 22, 2015, 5:36 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Zameer 

Re: Review Request 39563: Ignore serverInfo on the client side.

2015-10-22 Thread Kevin Sweeney

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


Elaborating - as far as I can see it the only sensible implementation of this 
feature would be for the client to set the version in the *request* and for the 
scheduler to check it and decide whether it supported the requested protocol 
version before processing it, and for the *scheduler* to return something like 
HTTP's `501 Not Implemented`. Otherwise there are no compatibility guarantees 
whatsoever. IMO `ServerInfo` should be much more like HTTP's `Server` header - 
additional information the server can provide about itself.

- Kevin Sweeney


On Oct. 22, 2015, 2:35 p.m., Kevin Sweeney wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39563/
> ---
> 
> (Updated Oct. 22, 2015, 2:35 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Zameer Manji.
> 
> 
> Bugs: AURORA-1522
> https://issues.apache.org/jira/browse/AURORA-1522
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Ignore serverInfo on the client side.
> 
> The design of this check is flawed - the client has already sent an RPC to 
> the scheduler and received a response for it, meaning the request has already 
> been processed and this check only serves to ignore its results.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/client/api/scheduler_client.py 
> 8e91788d8cb69ef21df6b045cd07f8cb111b95b3 
>   src/test/python/apache/aurora/client/api/test_scheduler_client.py 
> 10e8ebb4a12cc39d296cfde64ed9b01119d0aac3 
> 
> Diff: https://reviews.apache.org/r/39563/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kevin Sweeney
> 
>



Re: Review Request 39563: Ignore serverInfo on the client side.

2015-10-22 Thread Aurora ReviewBot

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

Ship it!


Master (4eeec7a) 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 Oct. 22, 2015, 9:35 p.m., Kevin Sweeney wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39563/
> ---
> 
> (Updated Oct. 22, 2015, 9:35 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Zameer Manji.
> 
> 
> Bugs: AURORA-1522
> https://issues.apache.org/jira/browse/AURORA-1522
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Ignore serverInfo on the client side.
> 
> The design of this check is flawed - the client has already sent an RPC to 
> the scheduler and received a response for it, meaning the request has already 
> been processed and this check only serves to ignore its results.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/client/api/scheduler_client.py 
> 8e91788d8cb69ef21df6b045cd07f8cb111b95b3 
>   src/test/python/apache/aurora/client/api/test_scheduler_client.py 
> 10e8ebb4a12cc39d296cfde64ed9b01119d0aac3 
> 
> Diff: https://reviews.apache.org/r/39563/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kevin Sweeney
> 
>



Re: Review Request 39563: Ignore serverInfo on the client side.

2015-10-22 Thread Bill Farner


> On Oct. 22, 2015, 11:02 a.m., Maxim Khutornenko wrote:
> > src/main/python/apache/aurora/client/api/scheduler_client.py, lines 313-315
> > 
> >
> > This change suggests we are effectively dropping the support for this 
> > feature. I agree the design is flawed but probably not worth changing it 
> > now in view of REST API refactoring though. 
> > 
> > How about setting the version info in auth interceptor instead?
> 
> Kevin Sweeney wrote:
> I plan to follow up with a patch to do that as well (it's slightly more 
> involved) but I see no reason to keep this feature in place.
> 
> Maxim Khutornenko wrote:
> If you want to remove it I suggest file a ticket and address it 
> separately (with proper schema deprecation ticket, NEWS note and etc.).
> 
> Zameer Manji wrote:
> I'm going to agree with Maxim. We should remove it and lets follow our 
> schema deprecation policy.

I'm in favor of both - remove the check, start the deprecation.  Progress in 
the direction we want to go.


- Bill


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


On Oct. 22, 2015, 2:35 p.m., Kevin Sweeney wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39563/
> ---
> 
> (Updated Oct. 22, 2015, 2:35 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Zameer Manji.
> 
> 
> Bugs: AURORA-1522
> https://issues.apache.org/jira/browse/AURORA-1522
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Ignore serverInfo on the client side.
> 
> The design of this check is flawed - the client has already sent an RPC to 
> the scheduler and received a response for it, meaning the request has already 
> been processed and this check only serves to ignore its results.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/client/api/scheduler_client.py 
> 8e91788d8cb69ef21df6b045cd07f8cb111b95b3 
>   src/test/python/apache/aurora/client/api/test_scheduler_client.py 
> 10e8ebb4a12cc39d296cfde64ed9b01119d0aac3 
> 
> Diff: https://reviews.apache.org/r/39563/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kevin Sweeney
> 
>



Re: Review Request 39563: Ignore serverInfo on the client side.

2015-10-22 Thread Kevin Sweeney


> On Oct. 22, 2015, 2:41 p.m., Kevin Sweeney wrote:
> > Elaborating - as far as I can see it the only sensible implementation of 
> > this feature would be for the client to set the version in the *request* 
> > and for the scheduler to check it and decide whether it supported the 
> > requested protocol version before processing it, and for the *scheduler* to 
> > return something like HTTP's `501 Not Implemented`. Otherwise there are no 
> > compatibility guarantees whatsoever. IMO `ServerInfo` should be much more 
> > like HTTP's `Server` header - additional information the server can provide 
> > about itself.

This fixes compatibility with the released 0.9.0 scheduler though, which is the 
subject of the linked ticket. As noted there's no reason to crash here. A 
scheduler change will not take effect until 0.10.0.


- Kevin


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


On Oct. 22, 2015, 2:35 p.m., Kevin Sweeney wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39563/
> ---
> 
> (Updated Oct. 22, 2015, 2:35 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Zameer Manji.
> 
> 
> Bugs: AURORA-1522
> https://issues.apache.org/jira/browse/AURORA-1522
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Ignore serverInfo on the client side.
> 
> The design of this check is flawed - the client has already sent an RPC to 
> the scheduler and received a response for it, meaning the request has already 
> been processed and this check only serves to ignore its results.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/client/api/scheduler_client.py 
> 8e91788d8cb69ef21df6b045cd07f8cb111b95b3 
>   src/test/python/apache/aurora/client/api/test_scheduler_client.py 
> 10e8ebb4a12cc39d296cfde64ed9b01119d0aac3 
> 
> Diff: https://reviews.apache.org/r/39563/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kevin Sweeney
> 
>



Re: Review Request 39563: Ignore serverInfo on the client side.

2015-10-22 Thread Maxim Khutornenko


> On Oct. 22, 2015, 6:02 p.m., Maxim Khutornenko wrote:
> > src/main/python/apache/aurora/client/api/scheduler_client.py, lines 313-315
> > 
> >
> > This change suggests we are effectively dropping the support for this 
> > feature. I agree the design is flawed but probably not worth changing it 
> > now in view of REST API refactoring though. 
> > 
> > How about setting the version info in auth interceptor instead?
> 
> Kevin Sweeney wrote:
> I plan to follow up with a patch to do that as well (it's slightly more 
> involved) but I see no reason to keep this feature in place.

If you want to remove it I suggest file a ticket and address it separately 
(with proper schema deprecation ticket, NEWS note and etc.).


- Maxim


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


On Oct. 22, 2015, 9:35 p.m., Kevin Sweeney wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39563/
> ---
> 
> (Updated Oct. 22, 2015, 9:35 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Zameer Manji.
> 
> 
> Bugs: AURORA-1522
> https://issues.apache.org/jira/browse/AURORA-1522
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Ignore serverInfo on the client side.
> 
> The design of this check is flawed - the client has already sent an RPC to 
> the scheduler and received a response for it, meaning the request has already 
> been processed and this check only serves to ignore its results.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/client/api/scheduler_client.py 
> 8e91788d8cb69ef21df6b045cd07f8cb111b95b3 
>   src/test/python/apache/aurora/client/api/test_scheduler_client.py 
> 10e8ebb4a12cc39d296cfde64ed9b01119d0aac3 
> 
> Diff: https://reviews.apache.org/r/39563/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kevin Sweeney
> 
>



Re: Review Request 39563: Ignore serverInfo on the client side.

2015-10-22 Thread Bill Farner

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

Ship it!


I'm fine with this patch as-is.  No great reason to keep a feature that hasn't 
had good follow-through.  It's in the way, let's knock it down.  I agree that 
the thrift fields should be removed, but would prefer to see that in a 
follow-up ticket.

- Bill Farner


On Oct. 22, 2015, 2:35 p.m., Kevin Sweeney wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39563/
> ---
> 
> (Updated Oct. 22, 2015, 2:35 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Zameer Manji.
> 
> 
> Bugs: AURORA-1522
> https://issues.apache.org/jira/browse/AURORA-1522
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Ignore serverInfo on the client side.
> 
> The design of this check is flawed - the client has already sent an RPC to 
> the scheduler and received a response for it, meaning the request has already 
> been processed and this check only serves to ignore its results.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/client/api/scheduler_client.py 
> 8e91788d8cb69ef21df6b045cd07f8cb111b95b3 
>   src/test/python/apache/aurora/client/api/test_scheduler_client.py 
> 10e8ebb4a12cc39d296cfde64ed9b01119d0aac3 
> 
> Diff: https://reviews.apache.org/r/39563/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kevin Sweeney
> 
>



Re: Review Request 39563: Ignore serverInfo on the client side.

2015-10-22 Thread Maxim Khutornenko

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

Ship it!


Ship It!

- Maxim Khutornenko


On Oct. 22, 2015, 9:35 p.m., Kevin Sweeney wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39563/
> ---
> 
> (Updated Oct. 22, 2015, 9:35 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Zameer Manji.
> 
> 
> Bugs: AURORA-1522
> https://issues.apache.org/jira/browse/AURORA-1522
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Ignore serverInfo on the client side.
> 
> The design of this check is flawed - the client has already sent an RPC to 
> the scheduler and received a response for it, meaning the request has already 
> been processed and this check only serves to ignore its results.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/client/api/scheduler_client.py 
> 8e91788d8cb69ef21df6b045cd07f8cb111b95b3 
>   src/test/python/apache/aurora/client/api/test_scheduler_client.py 
> 10e8ebb4a12cc39d296cfde64ed9b01119d0aac3 
> 
> Diff: https://reviews.apache.org/r/39563/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kevin Sweeney
> 
>



Re: Review Request 39563: Ignore serverInfo on the client side.

2015-10-22 Thread Kevin Sweeney

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

(Updated Oct. 22, 2015, 4:27 p.m.)


Review request for Aurora, Maxim Khutornenko and Zameer Manji.


Changes
---

Remove bug id


Repository: aurora


Description
---

Ignore serverInfo on the client side.

The design of this check is flawed - the client has already sent an RPC to the 
scheduler and received a response for it, meaning the request has already been 
processed and this check only serves to ignore its results.


Diffs
-

  src/main/python/apache/aurora/client/api/scheduler_client.py 
7aa061917c40903bd2f2582a4e928cdfbea81273 
  src/test/python/apache/aurora/client/api/test_scheduler_client.py 
60d222f49e594fd7e5363da445fc57ddf8dc4aba 

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


Testing
---


Thanks,

Kevin Sweeney



Re: Review Request 39563: Ignore serverInfo on the client side.

2015-10-22 Thread Maxim Khutornenko


> On Oct. 22, 2015, 6:02 p.m., Maxim Khutornenko wrote:
> > src/main/python/apache/aurora/client/api/scheduler_client.py, lines 313-315
> > 
> >
> > This change suggests we are effectively dropping the support for this 
> > feature. I agree the design is flawed but probably not worth changing it 
> > now in view of REST API refactoring though. 
> > 
> > How about setting the version info in auth interceptor instead?
> 
> Kevin Sweeney wrote:
> I plan to follow up with a patch to do that as well (it's slightly more 
> involved) but I see no reason to keep this feature in place.
> 
> Maxim Khutornenko wrote:
> If you want to remove it I suggest file a ticket and address it 
> separately (with proper schema deprecation ticket, NEWS note and etc.).
> 
> Zameer Manji wrote:
> I'm going to agree with Maxim. We should remove it and lets follow our 
> schema deprecation policy.
> 
> Bill Farner wrote:
> I'm in favor of both - remove the check, start the deprecation.  Progress 
> in the direction we want to go.

I am fine with the change as long as it's not linked to AURORA-1522 as I 
understand there will be a follow up change to actually address AURORA-1522.


- Maxim


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


On Oct. 22, 2015, 9:35 p.m., Kevin Sweeney wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39563/
> ---
> 
> (Updated Oct. 22, 2015, 9:35 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Zameer Manji.
> 
> 
> Bugs: AURORA-1522
> https://issues.apache.org/jira/browse/AURORA-1522
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Ignore serverInfo on the client side.
> 
> The design of this check is flawed - the client has already sent an RPC to 
> the scheduler and received a response for it, meaning the request has already 
> been processed and this check only serves to ignore its results.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/client/api/scheduler_client.py 
> 8e91788d8cb69ef21df6b045cd07f8cb111b95b3 
>   src/test/python/apache/aurora/client/api/test_scheduler_client.py 
> 10e8ebb4a12cc39d296cfde64ed9b01119d0aac3 
> 
> Diff: https://reviews.apache.org/r/39563/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kevin Sweeney
> 
>



Re: Review Request 39572: Remove callable check.

2015-10-22 Thread Zameer Manji

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

Ship it!


Ship It!

- Zameer Manji


On Oct. 22, 2015, 4:42 p.m., Kevin Sweeney wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39572/
> ---
> 
> (Updated Oct. 22, 2015, 4:42 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Zameer Manji.
> 
> 
> Bugs: AURORA-1524
> https://issues.apache.org/jira/browse/AURORA-1524
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Remove callable check.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/common/auth/auth_module_manager.py 
> 07a2730a0d4221cd989e85e40daf117a2da16cb4 
> 
> Diff: https://reviews.apache.org/r/39572/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kevin Sweeney
> 
>



Re: Review Request 39563: Ignore serverInfo on the client side.

2015-10-22 Thread Zameer Manji

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

Ship it!


After talking with Kevin offline, I think this is the way to go so long as we 
also start the deprecation process.

- Zameer Manji


On Oct. 22, 2015, 2:35 p.m., Kevin Sweeney wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39563/
> ---
> 
> (Updated Oct. 22, 2015, 2:35 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Zameer Manji.
> 
> 
> Bugs: AURORA-1522
> https://issues.apache.org/jira/browse/AURORA-1522
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Ignore serverInfo on the client side.
> 
> The design of this check is flawed - the client has already sent an RPC to 
> the scheduler and received a response for it, meaning the request has already 
> been processed and this check only serves to ignore its results.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/client/api/scheduler_client.py 
> 8e91788d8cb69ef21df6b045cd07f8cb111b95b3 
>   src/test/python/apache/aurora/client/api/test_scheduler_client.py 
> 10e8ebb4a12cc39d296cfde64ed9b01119d0aac3 
> 
> Diff: https://reviews.apache.org/r/39563/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kevin Sweeney
> 
>



Re: Review Request 39563: Ignore serverInfo on the client side.

2015-10-22 Thread Kevin Sweeney

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

(Updated Oct. 22, 2015, 4:26 p.m.)


Review request for Aurora, Maxim Khutornenko and Zameer Manji.


Changes
---

rebase


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


Repository: aurora


Description
---

Ignore serverInfo on the client side.

The design of this check is flawed - the client has already sent an RPC to the 
scheduler and received a response for it, meaning the request has already been 
processed and this check only serves to ignore its results.


Diffs (updated)
-

  src/main/python/apache/aurora/client/api/scheduler_client.py 
7aa061917c40903bd2f2582a4e928cdfbea81273 
  src/test/python/apache/aurora/client/api/test_scheduler_client.py 
60d222f49e594fd7e5363da445fc57ddf8dc4aba 

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


Testing
---


Thanks,

Kevin Sweeney



Re: Review Request 39563: Ignore serverInfo on the client side.

2015-10-22 Thread Kevin Sweeney

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

(Updated Oct. 22, 2015, 4:58 p.m.)


Review request for Aurora, Maxim Khutornenko and Zameer Manji.


Changes
---

Rebase correctly.


Repository: aurora


Description
---

Ignore serverInfo on the client side.

The design of this check is flawed - the client has already sent an RPC to the 
scheduler and received a response for it, meaning the request has already been 
processed and this check only serves to ignore its results.


Diffs (updated)
-

  src/main/python/apache/aurora/client/api/scheduler_client.py 
7aa061917c40903bd2f2582a4e928cdfbea81273 
  src/test/python/apache/aurora/client/api/test_scheduler_client.py 
60d222f49e594fd7e5363da445fc57ddf8dc4aba 

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


Testing
---


Thanks,

Kevin Sweeney