Re: Review Request 54957: Add option to not retry api calls to the scheduler.

[Mehrdad Nurolahzade]

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



I see three catgories of timeouts here:

(a) Aurora scheduler cannot be reached

If a connection between Aurora client and scheduler has not been established 
yet or has been invalidated/terminated due to error (e.g., transport exception) 
then client makes an attempt to stablish the connection before submitting an 
operation to the scheduler. If stablishing the connection takes longer than 1 
minute, a `TimeoutError` is raised in the following context in 
`method_wrapper()` that is then trapped, leading to an automatic retry of the 
operation.

```
@functools.wraps(method)
def method_wrapper(*args):
  with self._lock:
start = time.time()
while not self._terminating.is_set() and (
time.time() - start) < self.RPC_MAXIMUM_WAIT.as_(Time.SECONDS):

  try:
// self.client() raises self.TimeoutError after 1 minute if it 
cannot establish a connection
method = getattr(self.client(), method_name)
if not callable(method):
  return method
// raises TTransport.TTransportException for HTTP response code in 
the ranges 4XX and 5XX
resp = method(*args)
if resp is not None and resp.responseCode == 
ResponseCode.ERROR_TRANSIENT:
  raise self.TransientError(", ".join(
  [m.message for m in resp.details] if resp.details else []))   
 
  ...
  except (TTransport.TTransportException, self.TimeoutError, 
self.TransientError) as e:
if not self._terminating.is_set():
  log.warning('Connection error with scheduler: %s, 
reconnecting...' % e)
  self.invalidate()
  self._terminating.wait(self.RPC_RETRY_INTERVAL.as_(Time.SECONDS))
...
```

(b) Transport exception occured in the communication

A transport exception (e.g., HTTP 503 Service Temporarily Unavailable) can be 
raised in the communication link between Aurora and scheduler in 
`TTransportException.flush()`. 

```
  def flush(self):
...
try:
  response = self._session.post(
  self.__uri,
  data=data,
  timeout=self.__timeout,
  auth=self.__auth)
  response.raise_for_status()
...
```

This `TTransport.TTransportException` is trapped in 
`SchedulerClient.method_wrapper()`, leading to an automatic retry of the 
operation.

(c) Transient state error in scheduler

If Aurora scheduler discovers that its storage is not in the `READY` state to 
process a storage operation (i.e., read, write, or snapshot) it throws 
`Storage.TransientException`. This is caught by `LoggingInterceptor` and 
translated to a thrift response with `ResponseCode.ERROR_TRANSIENT`.

```
  @Override
  public Object invoke(MethodInvocation invocation) throws Throwable {
...
Response response = null;
try {
  // casting is safe, interception happens on methods that return Response 
or its subclasses
  response = (Response) invocation.proceed();
} catch (Storage.TransientStorageException e) {
  LOG.warn("Uncaught transient exception while handling {}({})", 
methodName, messageArgs, e);
  response = Responses.addMessage(Responses.empty(), 
ResponseCode.ERROR_TRANSIENT, e);
} 
...
```

Aurora client receives `ResponseCode.ERROR_TRANSIENT` and raises 
`TransientError` which is trapped in `SchedulerClient.method_wrapper()`, 
leading to an automatic retry of the operation.

Based on the above analysis, I believe:
- (a) is always safe to retry; it just establishes a connection with the 
scheduler.
- (b) is not safe to retry; the operation may or may not have been processed by 
the scheduler.
- (c) is always safe to retry; previous attempt to modify storage has failed.

- Mehrdad Nurolahzade


On March 24, 2017, 11:30 a.m., Karthik Anantha Padmanabhan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54957/
> ---
> 
> (Updated March 24, 2017, 11:30 a.m.)
> 
> 
> Review request for Aurora, David McLaughlin, Joshua Cohen, Santhosh Kumar 
> Shanmugham, and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This diff adds an option to not retry api calls to the scheduler. For some of 
> the non-idempotent operations we would like to not automatically retry. This 
> patch makes this functionality available only to the `schedule_backup_now` 
> command.
> 
> If there is consensus, this can be added to all commands as well.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/admin/admin.py 
> 070c348d2ca5db1edecf832efd9aa5481bddaa4b 
>   

Re: Review Request 54957: Add option to not retry api calls to the scheduler.

[Santhosh Kumar Shanmugham]

> On March 27, 2017, 3:22 p.m., Stephan Erb wrote:
> > src/main/python/apache/aurora/client/api/scheduler_client.py
> > Line 324 (original), 324 (patched)
> > 
> >
> > Could you please elaborate why TransportException and TransientError 
> > require special retry handling, whereas a TimeoutError can always be 
> > retried? Sorry if I have missed some previous discussion.

Seeing that there is some subtly in the choice to retry - can you capture the 
same in a comment as well? Thanks.


- Santhosh Kumar


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


On March 24, 2017, 11:30 a.m., Karthik Anantha Padmanabhan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54957/
> ---
> 
> (Updated March 24, 2017, 11:30 a.m.)
> 
> 
> Review request for Aurora, David McLaughlin, Joshua Cohen, Santhosh Kumar 
> Shanmugham, and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This diff adds an option to not retry api calls to the scheduler. For some of 
> the non-idempotent operations we would like to not automatically retry. This 
> patch makes this functionality available only to the `schedule_backup_now` 
> command.
> 
> If there is consensus, this can be added to all commands as well.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/admin/admin.py 
> 070c348d2ca5db1edecf832efd9aa5481bddaa4b 
>   src/main/python/apache/aurora/client/api/__init__.py 
> e1dde638bd1d686269fbcd88cb083a52e7f5dbfc 
>   src/main/python/apache/aurora/client/api/scheduler_client.py 
> 9bbfece012e48e0b1752bbefd25c89e04d312cf6 
>   src/test/python/apache/aurora/client/api/test_scheduler_client.py 
> f6018caa4f431e85a9e9ff203ac3d4b6c33f40ef 
> 
> 
> Diff: https://reviews.apache.org/r/54957/diff/6/
> 
> 
> Testing
> ---
> 
> * Manual testing
> * ./build-support/jenkins/build.sh passes
> 
> 
> Thanks,
> 
> Karthik Anantha Padmanabhan
> 
>

Re: Review Request 54957: Add option to not retry api calls to the scheduler.

[Stephan Erb]

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




src/main/python/apache/aurora/client/api/scheduler_client.py
Line 324 (original), 324 (patched)


Could you please elaborate why TransportException and TransientError 
require special retry handling, whereas a TimeoutError can always be retried? 
Sorry if I have missed some previous discussion.


- Stephan Erb


On March 24, 2017, 7:30 p.m., Karthik Anantha Padmanabhan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54957/
> ---
> 
> (Updated March 24, 2017, 7:30 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin, Joshua Cohen, Santhosh Kumar 
> Shanmugham, and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This diff adds an option to not retry api calls to the scheduler. For some of 
> the non-idempotent operations we would like to not automatically retry. This 
> patch makes this functionality available only to the `schedule_backup_now` 
> command.
> 
> If there is consensus, this can be added to all commands as well.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/admin/admin.py 
> 070c348d2ca5db1edecf832efd9aa5481bddaa4b 
>   src/main/python/apache/aurora/client/api/__init__.py 
> e1dde638bd1d686269fbcd88cb083a52e7f5dbfc 
>   src/main/python/apache/aurora/client/api/scheduler_client.py 
> 9bbfece012e48e0b1752bbefd25c89e04d312cf6 
>   src/test/python/apache/aurora/client/api/test_scheduler_client.py 
> f6018caa4f431e85a9e9ff203ac3d4b6c33f40ef 
> 
> 
> Diff: https://reviews.apache.org/r/54957/diff/6/
> 
> 
> Testing
> ---
> 
> * Manual testing
> * ./build-support/jenkins/build.sh passes
> 
> 
> Thanks,
> 
> Karthik Anantha Padmanabhan
> 
>

Re: Review Request 54957: Add option to not retry api calls to the scheduler.

[Aurora ReviewBot]

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


Ship it!




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

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

- Aurora ReviewBot


On March 24, 2017, 6:30 p.m., Karthik Anantha Padmanabhan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54957/
> ---
> 
> (Updated March 24, 2017, 6:30 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin, Joshua Cohen, Santhosh Kumar 
> Shanmugham, and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This diff adds an option to not retry api calls to the scheduler. For some of 
> the non-idempotent operations we would like to not automatically retry. This 
> patch makes this functionality available only to the `schedule_backup_now` 
> command.
> 
> If there is consensus, this can be added to all commands as well.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/admin/admin.py 
> 070c348d2ca5db1edecf832efd9aa5481bddaa4b 
>   src/main/python/apache/aurora/client/api/__init__.py 
> e1dde638bd1d686269fbcd88cb083a52e7f5dbfc 
>   src/main/python/apache/aurora/client/api/scheduler_client.py 
> 9bbfece012e48e0b1752bbefd25c89e04d312cf6 
>   src/test/python/apache/aurora/client/api/test_scheduler_client.py 
> f6018caa4f431e85a9e9ff203ac3d4b6c33f40ef 
> 
> 
> Diff: https://reviews.apache.org/r/54957/diff/6/
> 
> 
> Testing
> ---
> 
> * Manual testing
> * ./build-support/jenkins/build.sh passes
> 
> 
> Thanks,
> 
> Karthik Anantha Padmanabhan
> 
>

Re: Review Request 54957: Add option to not retry api calls to the scheduler.

[Aurora ReviewBot]

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



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

  Running setup.py bdist_wheel for twitter.common.log: finished with status 
'done'
  Stored in directory: 
/home/jenkins/jenkins-slave/workspace/AuroraBot/.home/.cache/pip/wheels/80/e3/3a/297d8950fcbd822ab5a6a62175818fab38b668cc5a86dbd0b0
  Running setup.py bdist_wheel for pycparser: started
  Running setup.py bdist_wheel for pycparser: finished with status 'done'
  Stored in directory: 
/home/jenkins/jenkins-slave/workspace/AuroraBot/.home/.cache/pip/wheels/a8/0b/41/dc95621f9d3a0da7bc191b8a71f0e8182ffd3cc5f33ac55005
  Running setup.py bdist_wheel for twitter.common.options: started
  Running setup.py bdist_wheel for twitter.common.options: finished with status 
'done'
  Stored in directory: 
/home/jenkins/jenkins-slave/workspace/AuroraBot/.home/.cache/pip/wheels/17/41/80/c4811d8c1c7ca7007e520c3399872fc340f45c3a26a6a23e6a
Successfully built pantsbuild.pants twitter.common.collections setproctitle 
ansicolors pathspec scandir twitter.common.dirutil psutil pystache docutils 
Markdown Pygments twitter.common.confluence coverage lmdb pywatchman 
twitter.common.lang twitter.common.log pycparser twitter.common.options
Installing collected packages: twitter.common.lang, twitter.common.collections, 
setproctitle, setuptools, six, ansicolors, pyparsing, packaging, pathspec, 
scandir, twitter.common.dirutil, psutil, requests, pystache, pex, docutils, 
Markdown, Pygments, twitter.common.options, twitter.common.log, 
twitter.common.confluence, monotonic, fasteners, coverage, pycparser, cffi, 
lmdb, pywatchman, futures, pantsbuild.pants
  Found existing installation: setuptools 21.2.1
Uninstalling setuptools-21.2.1:
  Successfully uninstalled setuptools-21.2.1
Successfully installed Markdown-2.1.1 Pygments-1.4 ansicolors-1.0.2 cffi-1.7.0 
coverage-3.7.1 docutils-0.12 fasteners-0.14.1 futures-3.0.5 lmdb-0.89 
monotonic-1.3 packaging-16.7 pantsbuild.pants-1.3.0.dev3 pathspec-0.3.4 
pex-1.1.16 psutil-4.3.0 pycparser-2.17 pyparsing-2.2.0 pystache-0.5.3 
pywatchman-1.3.0 requests-2.5.3 scandir-1.2 setproctitle-1.1.10 
setuptools-30.0.0 six-1.10.0 twitter.common.collections-0.3.9 
twitter.common.confluence-0.3.9 twitter.common.dirutil-0.3.9 
twitter.common.lang-0.3.9 twitter.common.log-0.3.9 twitter.common.options-0.3.9
You are using pip version 8.1.2, however version 9.0.1 is available.
You should consider upgrading via the 'pip install --upgrade pip' command.

04:47:22 00:00 [main]
   (To run a reporting server: ./pants server)
04:47:22 00:00   [setup]
04:47:22 00:00 [parse]
   Executing tasks in goals: compile
04:47:23 00:01   [compile]
04:47:23 00:01 [compile-prep-command]
04:47:23 00:01   [prep_command]
04:47:25 00:03 [compile]
04:47:25 00:03 [python-eval]
04:47:25 00:03 [pythonstyle]
04:47:26 00:04   [cache]  
   No cached artifacts for 42 targets.
   Invalidated 42 targets.
F841:ERROR   src/main/python/apache/aurora/admin/admin.py:274 local variable 
'options' is assigned to but never used
 |  options = app.get_options()


FAILURE: 1 Python Style issues found. For import order related issues, please 
try `./pants fmt.isort `


04:47:49 00:27   [complete]
   FAILURE


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

- Aurora ReviewBot


On March 24, 2017, 4:38 a.m., Karthik Anantha Padmanabhan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54957/
> ---
> 
> (Updated March 24, 2017, 4:38 a.m.)
> 
> 
> Review request for Aurora, David McLaughlin, Joshua Cohen, Santhosh Kumar 
> Shanmugham, and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This diff adds an option to not retry api calls to the scheduler. For some of 
> the non-idempotent operations we would like to not automatically retry. This 
> patch makes this functionality available only to the `schedule_backup_now` 
> command.
> 
> If there is consensus, this can be added to all commands as well.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/admin/admin.py 
> 070c348d2ca5db1edecf832efd9aa5481bddaa4b 
>   src/main/python/apache/aurora/client/api/__init__.py 
> e1dde638bd1d686269fbcd88cb083a52e7f5dbfc 
>   src/main/python/apache/aurora/client/api/scheduler_client.py 
> 9bbfece012e48e0b1752bbefd25c89e04d312cf6 
>   src/test/python/apache/aurora/client/api/test_scheduler_client.py 
> f6018caa4f431e85a9e9ff203ac3d4b6c33f40ef 
> 
> 
> 

Re: Review Request 54957: Add option to not retry api calls to the scheduler.

[Karthik Anantha Padmanabhan]

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

(Updated March 24, 2017, 4:38 a.m.)


Review request for Aurora, David McLaughlin, Joshua Cohen, Santhosh Kumar 
Shanmugham, and Zameer Manji.


Repository: aurora


Description
---

This diff adds an option to not retry api calls to the scheduler. For some of 
the non-idempotent operations we would like to not automatically retry. This 
patch makes this functionality available only to the `schedule_backup_now` 
command.

If there is consensus, this can be added to all commands as well.


Diffs (updated)
-

  src/main/python/apache/aurora/admin/admin.py 
070c348d2ca5db1edecf832efd9aa5481bddaa4b 
  src/main/python/apache/aurora/client/api/__init__.py 
e1dde638bd1d686269fbcd88cb083a52e7f5dbfc 
  src/main/python/apache/aurora/client/api/scheduler_client.py 
9bbfece012e48e0b1752bbefd25c89e04d312cf6 
  src/test/python/apache/aurora/client/api/test_scheduler_client.py 
f6018caa4f431e85a9e9ff203ac3d4b6c33f40ef 


Diff: https://reviews.apache.org/r/54957/diff/5/

Changes: https://reviews.apache.org/r/54957/diff/4-5/


Testing
---

* Manual testing
* ./build-support/jenkins/build.sh passes


Thanks,

Karthik Anantha Padmanabhan

Re: Review Request 54957: Add option to not retry api calls to the scheduler.

[Karthik Anantha Padmanabhan]

> On March 23, 2017, 4:52 p.m., David McLaughlin wrote:
> > src/main/python/apache/aurora/admin/admin.py
> > Lines 244 (patched)
> > 
> >
> > Where is options.no_retry defined?

duh ! Bad push my bad.


- Karthik


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


On March 23, 2017, 4:23 p.m., Karthik Anantha Padmanabhan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54957/
> ---
> 
> (Updated March 23, 2017, 4:23 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin, Joshua Cohen, Santhosh Kumar 
> Shanmugham, and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This diff adds an option to not retry api calls to the scheduler. For some of 
> the non-idempotent operations we would like to not automatically retry. This 
> patch makes this functionality available only to the `schedule_backup_now` 
> command.
> 
> If there is consensus, this can be added to all commands as well.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/admin/admin.py 
> 070c348d2ca5db1edecf832efd9aa5481bddaa4b 
>   src/main/python/apache/aurora/client/api/__init__.py 
> e1dde638bd1d686269fbcd88cb083a52e7f5dbfc 
>   src/main/python/apache/aurora/client/api/scheduler_client.py 
> 9bbfece012e48e0b1752bbefd25c89e04d312cf6 
>   src/test/python/apache/aurora/client/api/test_scheduler_client.py 
> f6018caa4f431e85a9e9ff203ac3d4b6c33f40ef 
> 
> 
> Diff: https://reviews.apache.org/r/54957/diff/4/
> 
> 
> Testing
> ---
> 
> * Manual testing
> * ./build-support/jenkins/build.sh passes
> 
> 
> Thanks,
> 
> Karthik Anantha Padmanabhan
> 
>

Re: Review Request 54957: Add option to not retry api calls to the scheduler.

[David McLaughlin]

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




src/main/python/apache/aurora/admin/admin.py
Lines 244 (patched)


Where is options.no_retry defined?


- David McLaughlin


On March 23, 2017, 4:23 p.m., Karthik Anantha Padmanabhan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54957/
> ---
> 
> (Updated March 23, 2017, 4:23 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin, Joshua Cohen, Santhosh Kumar 
> Shanmugham, and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This diff adds an option to not retry api calls to the scheduler. For some of 
> the non-idempotent operations we would like to not automatically retry. This 
> patch makes this functionality available only to the `schedule_backup_now` 
> command.
> 
> If there is consensus, this can be added to all commands as well.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/admin/admin.py 
> 070c348d2ca5db1edecf832efd9aa5481bddaa4b 
>   src/main/python/apache/aurora/client/api/__init__.py 
> e1dde638bd1d686269fbcd88cb083a52e7f5dbfc 
>   src/main/python/apache/aurora/client/api/scheduler_client.py 
> 9bbfece012e48e0b1752bbefd25c89e04d312cf6 
>   src/test/python/apache/aurora/client/api/test_scheduler_client.py 
> f6018caa4f431e85a9e9ff203ac3d4b6c33f40ef 
> 
> 
> Diff: https://reviews.apache.org/r/54957/diff/4/
> 
> 
> Testing
> ---
> 
> * Manual testing
> * ./build-support/jenkins/build.sh passes
> 
> 
> Thanks,
> 
> Karthik Anantha Padmanabhan
> 
>

Re: Review Request 54957: Add option to not retry api calls to the scheduler.

[Aurora ReviewBot]

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


Ship it!




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

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

- Aurora ReviewBot


On March 23, 2017, 4:23 p.m., Karthik Anantha Padmanabhan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54957/
> ---
> 
> (Updated March 23, 2017, 4:23 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin, Joshua Cohen, Santhosh Kumar 
> Shanmugham, and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This diff adds an option to not retry api calls to the scheduler. For some of 
> the non-idempotent operations we would like to not automatically retry. This 
> patch makes this functionality available only to the `schedule_backup_now` 
> command.
> 
> If there is consensus, this can be added to all commands as well.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/admin/admin.py 
> 070c348d2ca5db1edecf832efd9aa5481bddaa4b 
>   src/main/python/apache/aurora/client/api/__init__.py 
> e1dde638bd1d686269fbcd88cb083a52e7f5dbfc 
>   src/main/python/apache/aurora/client/api/scheduler_client.py 
> 9bbfece012e48e0b1752bbefd25c89e04d312cf6 
>   src/test/python/apache/aurora/client/api/test_scheduler_client.py 
> f6018caa4f431e85a9e9ff203ac3d4b6c33f40ef 
> 
> 
> Diff: https://reviews.apache.org/r/54957/diff/4/
> 
> 
> Testing
> ---
> 
> * Manual testing
> * ./build-support/jenkins/build.sh passes
> 
> 
> Thanks,
> 
> Karthik Anantha Padmanabhan
> 
>

Re: Review Request 54957: Add option to not retry api calls to the scheduler.

[Karthik Anantha Padmanabhan]

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

(Updated March 23, 2017, 4:23 p.m.)


Review request for Aurora, David McLaughlin, Joshua Cohen, Santhosh Kumar 
Shanmugham, and Zameer Manji.


Changes
---

Style changes.


Repository: aurora


Description
---

This diff adds an option to not retry api calls to the scheduler. For some of 
the non-idempotent operations we would like to not automatically retry. This 
patch makes this functionality available only to the `schedule_backup_now` 
command.

If there is consensus, this can be added to all commands as well.


Diffs (updated)
-

  src/main/python/apache/aurora/admin/admin.py 
070c348d2ca5db1edecf832efd9aa5481bddaa4b 
  src/main/python/apache/aurora/client/api/__init__.py 
e1dde638bd1d686269fbcd88cb083a52e7f5dbfc 
  src/main/python/apache/aurora/client/api/scheduler_client.py 
9bbfece012e48e0b1752bbefd25c89e04d312cf6 
  src/test/python/apache/aurora/client/api/test_scheduler_client.py 
f6018caa4f431e85a9e9ff203ac3d4b6c33f40ef 


Diff: https://reviews.apache.org/r/54957/diff/4/

Changes: https://reviews.apache.org/r/54957/diff/3-4/


Testing
---

* Manual testing
* ./build-support/jenkins/build.sh passes


Thanks,

Karthik Anantha Padmanabhan

Re: Review Request 54957: Add option to not retry api calls to the scheduler.

[Aurora ReviewBot]

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



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

   (To run a reporting server: ./pants server)
08:38:11 00:00   [setup]
08:38:11 00:00 [parse]
   Executing tasks in goals: compile
08:38:11 00:00   [compile]
08:38:11 00:00 [compile-prep-command]
08:38:11 00:00   [prep_command]
08:38:14 00:03 [compile]
08:38:14 00:03 [python-eval]
08:38:14 00:03 [pythonstyle]
08:38:14 00:03   [cache]  
   No cached artifacts for 42 targets.
   Invalidated 42 targets.
T302:ERROR   
src/test/python/apache/aurora/client/api/test_scheduler_client.py:059 Expected 
2 blank lines, found 1
 |def test_coverage():

T301:ERROR   
src/test/python/apache/aurora/client/api/test_scheduler_client.py:598-599 
Expected 1 blank lines, found 2
 |  @mock.patch('apache.aurora.client.api.scheduler_client.SchedulerClient',
 |  spec=scheduler_client.SchedulerClient)

E302:ERROR   
PythonFile(src/test/python/apache/aurora/client/api/test_scheduler_client.py):059
 expected 2 blank lines, found 1
 |def test_coverage():

E303:ERROR   
PythonFile(src/test/python/apache/aurora/client/api/test_scheduler_client.py):598-599
 too many blank lines (2)
 |  @mock.patch('apache.aurora.client.api.scheduler_client.SchedulerClient',
 |  spec=scheduler_client.SchedulerClient)


T302:ERROR   src/main/python/apache/aurora/admin/aurora_admin.py:050 Expected 2 
blank lines, found 1
 |def proxy_main():

E302:ERROR   
PythonFile(src/main/python/apache/aurora/admin/aurora_admin.py):050 expected 2 
blank lines, found 1
 |def proxy_main():


FAILURE: 6 Python Style issues found. For import order related issues, please 
try `./pants fmt.isort `


08:38:39 00:28   [complete]
   FAILURE


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

- Aurora ReviewBot


On March 23, 2017, 8:23 a.m., Karthik Anantha Padmanabhan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54957/
> ---
> 
> (Updated March 23, 2017, 8:23 a.m.)
> 
> 
> Review request for Aurora, David McLaughlin, Joshua Cohen, Santhosh Kumar 
> Shanmugham, and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This diff adds an option to not retry api calls to the scheduler. For some of 
> the non-idempotent operations we would like to not automatically retry. This 
> patch makes this functionality available only to the `schedule_backup_now` 
> command.
> 
> If there is consensus, this can be added to all commands as well.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/admin/admin.py 
> 070c348d2ca5db1edecf832efd9aa5481bddaa4b 
>   src/main/python/apache/aurora/admin/aurora_admin.py 
> fbebbab8c827b5695042d18770d850e31fc38122 
>   src/main/python/apache/aurora/client/api/__init__.py 
> e1dde638bd1d686269fbcd88cb083a52e7f5dbfc 
>   src/main/python/apache/aurora/client/api/scheduler_client.py 
> 9bbfece012e48e0b1752bbefd25c89e04d312cf6 
>   src/test/python/apache/aurora/client/api/test_scheduler_client.py 
> f6018caa4f431e85a9e9ff203ac3d4b6c33f40ef 
> 
> 
> Diff: https://reviews.apache.org/r/54957/diff/3/
> 
> 
> Testing
> ---
> 
> * Manual testing
> * ./build-support/jenkins/build.sh passes
> 
> 
> Thanks,
> 
> Karthik Anantha Padmanabhan
> 
>

Re: Review Request 54957: Add option to not retry api calls to the scheduler.

[Karthik Anantha Padmanabhan]

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

(Updated March 23, 2017, 8:23 a.m.)


Review request for Aurora, David McLaughlin, Joshua Cohen, Santhosh Kumar 
Shanmugham, and Zameer Manji.


Repository: aurora


Description
---

This diff adds an option to not retry api calls to the scheduler. For some of 
the non-idempotent operations we would like to not automatically retry. This 
patch makes this functionality available only to the `schedule_backup_now` 
command.

If there is consensus, this can be added to all commands as well.


Diffs
-

  src/main/python/apache/aurora/admin/admin.py 
070c348d2ca5db1edecf832efd9aa5481bddaa4b 
  src/main/python/apache/aurora/admin/aurora_admin.py 
fbebbab8c827b5695042d18770d850e31fc38122 
  src/main/python/apache/aurora/client/api/__init__.py 
e1dde638bd1d686269fbcd88cb083a52e7f5dbfc 
  src/main/python/apache/aurora/client/api/scheduler_client.py 
9bbfece012e48e0b1752bbefd25c89e04d312cf6 
  src/test/python/apache/aurora/client/api/test_scheduler_client.py 
f6018caa4f431e85a9e9ff203ac3d4b6c33f40ef 


Diff: https://reviews.apache.org/r/54957/diff/3/


Testing (updated)
---

* Manual testing
* ./build-support/jenkins/build.sh passes


Thanks,

Karthik Anantha Padmanabhan

Re: Review Request 54957: Add option to not retry api calls to the scheduler.

[Karthik Anantha Padmanabhan]

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

(Updated March 23, 2017, 8:23 a.m.)


Review request for Aurora, David McLaughlin, Joshua Cohen, Santhosh Kumar 
Shanmugham, and Zameer Manji.


Changes
---

Rename tests.


Repository: aurora


Description
---

This diff adds an option to not retry api calls to the scheduler. For some of 
the non-idempotent operations we would like to not automatically retry. This 
patch makes this functionality available only to the `schedule_backup_now` 
command.

If there is consensus, this can be added to all commands as well.


Diffs (updated)
-

  src/main/python/apache/aurora/admin/admin.py 
070c348d2ca5db1edecf832efd9aa5481bddaa4b 
  src/main/python/apache/aurora/admin/aurora_admin.py 
fbebbab8c827b5695042d18770d850e31fc38122 
  src/main/python/apache/aurora/client/api/__init__.py 
e1dde638bd1d686269fbcd88cb083a52e7f5dbfc 
  src/main/python/apache/aurora/client/api/scheduler_client.py 
9bbfece012e48e0b1752bbefd25c89e04d312cf6 
  src/test/python/apache/aurora/client/api/test_scheduler_client.py 
f6018caa4f431e85a9e9ff203ac3d4b6c33f40ef 


Diff: https://reviews.apache.org/r/54957/diff/3/

Changes: https://reviews.apache.org/r/54957/diff/2-3/


Testing
---

* Manuall testing
* ./build-support/jenkins/build.sh passes


Thanks,

Karthik Anantha Padmanabhan

Re: Review Request 54957: Add option to not retry api calls to the scheduler.

[Karthik Anantha Padmanabhan]

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

(Updated March 23, 2017, 8:22 a.m.)


Review request for Aurora, David McLaughlin, Joshua Cohen, Santhosh Kumar 
Shanmugham, and Zameer Manji.


Changes
---

CR Comments.


Repository: aurora


Description
---

This diff adds an option to not retry api calls to the scheduler. For some of 
the non-idempotent operations we would like to not automatically retry. This 
patch makes this functionality available only to the `schedule_backup_now` 
command.

If there is consensus, this can be added to all commands as well.


Diffs (updated)
-

  src/main/python/apache/aurora/admin/admin.py 
070c348d2ca5db1edecf832efd9aa5481bddaa4b 
  src/main/python/apache/aurora/admin/aurora_admin.py 
fbebbab8c827b5695042d18770d850e31fc38122 
  src/main/python/apache/aurora/client/api/__init__.py 
e1dde638bd1d686269fbcd88cb083a52e7f5dbfc 
  src/main/python/apache/aurora/client/api/scheduler_client.py 
9bbfece012e48e0b1752bbefd25c89e04d312cf6 
  src/test/python/apache/aurora/client/api/test_scheduler_client.py 
f6018caa4f431e85a9e9ff203ac3d4b6c33f40ef 


Diff: https://reviews.apache.org/r/54957/diff/2/

Changes: https://reviews.apache.org/r/54957/diff/1-2/


Testing
---

* Manuall testing
* ./build-support/jenkins/build.sh passes


Thanks,

Karthik Anantha Padmanabhan

Re: Review Request 54957: Add option to not retry api calls to the scheduler.

[Stephan Erb]

> On Jan. 5, 2017, 12:54 a.m., Karthik Anantha Padmanabhan wrote:
> > I was going to wrap this up - but how do people feel about making the all 
> > endpoints "idempotent" by the following method ? Inlcude an 
> > "idempotency-token" along as part of the HTTP header. This token is locally 
> > cached for, say, an hour. Every request with the same token will return 
> > simply short circuit and not be processed. The retry logic to transport 
> > layer so that we can transparently add the idempotency tokens.
> 
> Santhosh Kumar Shanmugham wrote:
> Can you explain this in detail? Particularly flesh out the `HTTP headers` 
> (aren't we using a Thrift interface while speaking to the Scheduler?) and the 
> `locally cached` (have the Scheduler cache the `idempotency-token` or on the 
> client?) parts.
> 
> I like this approach better, which can fix the issue across all APIs.
> 
> Karthik Anantha Padmanabhan wrote:
> https://reviews.apache.org/r/55237/ is a WIP for idempotent requests. Yes 
> we still use HTTP as the transport but just stuff the thrift binary in the 
> body of a POST request.

I have to admit going down that route feels a little bit like overkill to me. 
Explicitly flagging the non-retryable operations as proposed by David will also 
get the job done and come with less complexity.


- Stephan


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


On Dec. 22, 2016, 1:06 a.m., Karthik Anantha Padmanabhan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54957/
> ---
> 
> (Updated Dec. 22, 2016, 1:06 a.m.)
> 
> 
> Review request for Aurora, David McLaughlin, Joshua Cohen, Santhosh Kumar 
> Shanmugham, and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This diff adds an option to not retry api calls to the scheduler. For some of 
> the non-idempotent operations we would like to not automatically retry. This 
> patch makes this functionality available only to the `schedule_backup_now` 
> command.
> 
> If there is consensus, this can be added to all commands as well.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/admin/admin.py 
> 070c348d2ca5db1edecf832efd9aa5481bddaa4b 
>   src/main/python/apache/aurora/admin/aurora_admin.py 
> fbebbab8c827b5695042d18770d850e31fc38122 
>   src/main/python/apache/aurora/client/api/__init__.py 
> e1dde638bd1d686269fbcd88cb083a52e7f5dbfc 
>   src/main/python/apache/aurora/client/api/scheduler_client.py 
> 9bbfece012e48e0b1752bbefd25c89e04d312cf6 
>   src/test/python/apache/aurora/client/api/test_scheduler_client.py 
> f6018caa4f431e85a9e9ff203ac3d4b6c33f40ef 
> 
> Diff: https://reviews.apache.org/r/54957/diff/
> 
> 
> Testing
> ---
> 
> * Manuall testing
> * ./build-support/jenkins/build.sh passes
> 
> 
> Thanks,
> 
> Karthik Anantha Padmanabhan
> 
>

Re: Review Request 54957: Add option to not retry api calls to the scheduler.

[Karthik Anantha Padmanabhan]

> On Jan. 4, 2017, 11:54 p.m., Karthik Anantha Padmanabhan wrote:
> > I was going to wrap this up - but how do people feel about making the all 
> > endpoints "idempotent" by the following method ? Inlcude an 
> > "idempotency-token" along as part of the HTTP header. This token is locally 
> > cached for, say, an hour. Every request with the same token will return 
> > simply short circuit and not be processed. The retry logic to transport 
> > layer so that we can transparently add the idempotency tokens.
> 
> Santhosh Kumar Shanmugham wrote:
> Can you explain this in detail? Particularly flesh out the `HTTP headers` 
> (aren't we using a Thrift interface while speaking to the Scheduler?) and the 
> `locally cached` (have the Scheduler cache the `idempotency-token` or on the 
> client?) parts.
> 
> I like this approach better, which can fix the issue across all APIs.

https://reviews.apache.org/r/55237/ is a WIP for idempotent requests. Yes we 
still use HTTP as the transport but just stuff the thrift binary in the body of 
a POST request.


- Karthik


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


On Dec. 22, 2016, 12:06 a.m., Karthik Anantha Padmanabhan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54957/
> ---
> 
> (Updated Dec. 22, 2016, 12:06 a.m.)
> 
> 
> Review request for Aurora, David McLaughlin, Joshua Cohen, Santhosh Kumar 
> Shanmugham, and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This diff adds an option to not retry api calls to the scheduler. For some of 
> the non-idempotent operations we would like to not automatically retry. This 
> patch makes this functionality available only to the `schedule_backup_now` 
> command.
> 
> If there is consensus, this can be added to all commands as well.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/admin/admin.py 
> 070c348d2ca5db1edecf832efd9aa5481bddaa4b 
>   src/main/python/apache/aurora/admin/aurora_admin.py 
> fbebbab8c827b5695042d18770d850e31fc38122 
>   src/main/python/apache/aurora/client/api/__init__.py 
> e1dde638bd1d686269fbcd88cb083a52e7f5dbfc 
>   src/main/python/apache/aurora/client/api/scheduler_client.py 
> 9bbfece012e48e0b1752bbefd25c89e04d312cf6 
>   src/test/python/apache/aurora/client/api/test_scheduler_client.py 
> f6018caa4f431e85a9e9ff203ac3d4b6c33f40ef 
> 
> Diff: https://reviews.apache.org/r/54957/diff/
> 
> 
> Testing
> ---
> 
> * Manuall testing
> * ./build-support/jenkins/build.sh passes
> 
> 
> Thanks,
> 
> Karthik Anantha Padmanabhan
> 
>

Re: Review Request 54957: Add option to not retry api calls to the scheduler.

[Santhosh Kumar Shanmugham]

> On Jan. 4, 2017, 3:54 p.m., Karthik Anantha Padmanabhan wrote:
> > I was going to wrap this up - but how do people feel about making the all 
> > endpoints "idempotent" by the following method ? Inlcude an 
> > "idempotency-token" along as part of the HTTP header. This token is locally 
> > cached for, say, an hour. Every request with the same token will return 
> > simply short circuit and not be processed. The retry logic to transport 
> > layer so that we can transparently add the idempotency tokens.

Can you explain this in detail? Particularly flesh out the `HTTP headers` 
(aren't we using a Thrift interface while speaking to the Scheduler?) and the 
`locally cached` (have the Scheduler cache the `idempotency-token` or on the 
client?) parts.

I like this approach better, which can fix the issue across all APIs.


- Santhosh Kumar


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


On Dec. 21, 2016, 4:06 p.m., Karthik Anantha Padmanabhan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54957/
> ---
> 
> (Updated Dec. 21, 2016, 4:06 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin, Joshua Cohen, Santhosh Kumar 
> Shanmugham, and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This diff adds an option to not retry api calls to the scheduler. For some of 
> the non-idempotent operations we would like to not automatically retry. This 
> patch makes this functionality available only to the `schedule_backup_now` 
> command.
> 
> If there is consensus, this can be added to all commands as well.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/admin/admin.py 
> 070c348d2ca5db1edecf832efd9aa5481bddaa4b 
>   src/main/python/apache/aurora/admin/aurora_admin.py 
> fbebbab8c827b5695042d18770d850e31fc38122 
>   src/main/python/apache/aurora/client/api/__init__.py 
> e1dde638bd1d686269fbcd88cb083a52e7f5dbfc 
>   src/main/python/apache/aurora/client/api/scheduler_client.py 
> 9bbfece012e48e0b1752bbefd25c89e04d312cf6 
>   src/test/python/apache/aurora/client/api/test_scheduler_client.py 
> f6018caa4f431e85a9e9ff203ac3d4b6c33f40ef 
> 
> Diff: https://reviews.apache.org/r/54957/diff/
> 
> 
> Testing
> ---
> 
> * Manuall testing
> * ./build-support/jenkins/build.sh passes
> 
> 
> Thanks,
> 
> Karthik Anantha Padmanabhan
> 
>

Re: Review Request 54957: Add option to not retry api calls to the scheduler.

[Stephan Erb]

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



This change is imporant enough to be mentioned in the release notes. Please add 
it there.


src/main/python/apache/aurora/client/api/scheduler_client.py (line 330)


Please add something like `log.warning('Error making api call: %s. Not 
retrying.', e)`. Otherwise we will swallow the exact exception.


- Stephan Erb


On Dec. 22, 2016, 1:06 a.m., Karthik Anantha Padmanabhan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54957/
> ---
> 
> (Updated Dec. 22, 2016, 1:06 a.m.)
> 
> 
> Review request for Aurora, David McLaughlin, Joshua Cohen, Santhosh Kumar 
> Shanmugham, and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This diff adds an option to not retry api calls to the scheduler. For some of 
> the non-idempotent operations we would like to not automatically retry. This 
> patch makes this functionality available only to the `schedule_backup_now` 
> command.
> 
> If there is consensus, this can be added to all commands as well.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/admin/admin.py 
> 070c348d2ca5db1edecf832efd9aa5481bddaa4b 
>   src/main/python/apache/aurora/admin/aurora_admin.py 
> fbebbab8c827b5695042d18770d850e31fc38122 
>   src/main/python/apache/aurora/client/api/__init__.py 
> e1dde638bd1d686269fbcd88cb083a52e7f5dbfc 
>   src/main/python/apache/aurora/client/api/scheduler_client.py 
> 9bbfece012e48e0b1752bbefd25c89e04d312cf6 
>   src/test/python/apache/aurora/client/api/test_scheduler_client.py 
> f6018caa4f431e85a9e9ff203ac3d4b6c33f40ef 
> 
> Diff: https://reviews.apache.org/r/54957/diff/
> 
> 
> Testing
> ---
> 
> * Manuall testing
> * ./build-support/jenkins/build.sh passes
> 
> 
> Thanks,
> 
> Karthik Anantha Padmanabhan
> 
>

Re: Review Request 54957: Add option to not retry api calls to the scheduler.

[David McLaughlin]

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



Is it even worth making this a controllable option? I think it's a bug that the 
existing client will retry a non-idempotent operation, especially one that can 
take down your cluster. 

Either way, we should be sure to default the option to False for anything that 
*shouldn't* be retried. And definitely for the admin options where the retries 
can take down your cluster.

- David McLaughlin


On Dec. 22, 2016, 12:06 a.m., Karthik Anantha Padmanabhan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54957/
> ---
> 
> (Updated Dec. 22, 2016, 12:06 a.m.)
> 
> 
> Review request for Aurora, David McLaughlin, Joshua Cohen, Santhosh Kumar 
> Shanmugham, and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This diff adds an option to not retry api calls to the scheduler. For some of 
> the non-idempotent operations we would like to not automatically retry. This 
> patch makes this functionality available only to the `schedule_backup_now` 
> command.
> 
> If there is consensus, this can be added to all commands as well.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/admin/admin.py 
> 070c348d2ca5db1edecf832efd9aa5481bddaa4b 
>   src/main/python/apache/aurora/admin/aurora_admin.py 
> fbebbab8c827b5695042d18770d850e31fc38122 
>   src/main/python/apache/aurora/client/api/__init__.py 
> e1dde638bd1d686269fbcd88cb083a52e7f5dbfc 
>   src/main/python/apache/aurora/client/api/scheduler_client.py 
> 9bbfece012e48e0b1752bbefd25c89e04d312cf6 
>   src/test/python/apache/aurora/client/api/test_scheduler_client.py 
> f6018caa4f431e85a9e9ff203ac3d4b6c33f40ef 
> 
> Diff: https://reviews.apache.org/r/54957/diff/
> 
> 
> Testing
> ---
> 
> * Manuall testing
> * ./build-support/jenkins/build.sh passes
> 
> 
> Thanks,
> 
> Karthik Anantha Padmanabhan
> 
>

Re: Review Request 54957: Add option to not retry api calls to the scheduler.

[Karthik Anantha Padmanabhan]

> On Dec. 22, 2016, 12:54 a.m., Karthik Anantha Padmanabhan wrote:
> > src/main/python/apache/aurora/admin/aurora_admin.py, line 55
> > 
> >
> > In the current implementation we retry only when  a `TimeoutError` is 
> > raised.
> > 
> > And we raise `TimeoutError` when trying to construct the client and 
> > connecting to the leader/ZK. And this might involve things like unable to 
> > resolve the leader url etc.

So when an API call timesout we do not retry.


- Karthik


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


On Dec. 22, 2016, 12:06 a.m., Karthik Anantha Padmanabhan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54957/
> ---
> 
> (Updated Dec. 22, 2016, 12:06 a.m.)
> 
> 
> Review request for Aurora, David McLaughlin, Joshua Cohen, Santhosh Kumar 
> Shanmugham, and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This diff adds an option to not retry api calls to the scheduler. For some of 
> the non-idempotent operations we would like to not automatically retry. This 
> patch makes this functionality available only to the `schedule_backup_now` 
> command.
> 
> If there is consensus, this can be added to all commands as well.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/admin/admin.py 
> 070c348d2ca5db1edecf832efd9aa5481bddaa4b 
>   src/main/python/apache/aurora/admin/aurora_admin.py 
> fbebbab8c827b5695042d18770d850e31fc38122 
>   src/main/python/apache/aurora/client/api/__init__.py 
> e1dde638bd1d686269fbcd88cb083a52e7f5dbfc 
>   src/main/python/apache/aurora/client/api/scheduler_client.py 
> 9bbfece012e48e0b1752bbefd25c89e04d312cf6 
>   src/test/python/apache/aurora/client/api/test_scheduler_client.py 
> f6018caa4f431e85a9e9ff203ac3d4b6c33f40ef 
> 
> Diff: https://reviews.apache.org/r/54957/diff/
> 
> 
> Testing
> ---
> 
> * Manuall testing
> * ./build-support/jenkins/build.sh passes
> 
> 
> Thanks,
> 
> Karthik Anantha Padmanabhan
> 
>

Re: Review Request 54957: Add option to not retry api calls to the scheduler.

[Karthik Anantha Padmanabhan]

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




src/main/python/apache/aurora/admin/aurora_admin.py (line 55)


In the current implementation we retry only when  a `TimeoutError` is 
raised.

And we raise `TimeoutError` when trying to construct the client and 
connecting to the leader/ZK. And this might involve things like unable to 
resolve the leader url etc.


- Karthik Anantha Padmanabhan


On Dec. 22, 2016, 12:06 a.m., Karthik Anantha Padmanabhan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54957/
> ---
> 
> (Updated Dec. 22, 2016, 12:06 a.m.)
> 
> 
> Review request for Aurora, David McLaughlin, Joshua Cohen, Santhosh Kumar 
> Shanmugham, and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This diff adds an option to not retry api calls to the scheduler. For some of 
> the non-idempotent operations we would like to not automatically retry. This 
> patch makes this functionality available only to the `schedule_backup_now` 
> command.
> 
> If there is consensus, this can be added to all commands as well.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/admin/admin.py 
> 070c348d2ca5db1edecf832efd9aa5481bddaa4b 
>   src/main/python/apache/aurora/admin/aurora_admin.py 
> fbebbab8c827b5695042d18770d850e31fc38122 
>   src/main/python/apache/aurora/client/api/__init__.py 
> e1dde638bd1d686269fbcd88cb083a52e7f5dbfc 
>   src/main/python/apache/aurora/client/api/scheduler_client.py 
> 9bbfece012e48e0b1752bbefd25c89e04d312cf6 
>   src/test/python/apache/aurora/client/api/test_scheduler_client.py 
> f6018caa4f431e85a9e9ff203ac3d4b6c33f40ef 
> 
> Diff: https://reviews.apache.org/r/54957/diff/
> 
> 
> Testing
> ---
> 
> * Manuall testing
> * ./build-support/jenkins/build.sh passes
> 
> 
> Thanks,
> 
> Karthik Anantha Padmanabhan
> 
>

Re: Review Request 54957: Add option to not retry api calls to the scheduler.

[Santhosh Kumar Shanmugham]

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




src/main/python/apache/aurora/admin/aurora_admin.py (line 51)


Since we are merely exposing control over something that is already 
present, can we just call this `--retry` and default to `true`. I feel like it 
will be easier to read.



src/main/python/apache/aurora/admin/aurora_admin.py (line 55)


This is not true. We still retry on timeouts.

It is misleading to have a switch `--no-retry` and still retry on some 
situtations. Maybe it should be all or nothing.



src/test/python/apache/aurora/client/api/test_scheduler_client.py (line 250)


This the same test as above isn't it. Can we test when we are no requesting 
retries?


- Santhosh Kumar Shanmugham


On Dec. 21, 2016, 4:06 p.m., Karthik Anantha Padmanabhan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54957/
> ---
> 
> (Updated Dec. 21, 2016, 4:06 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin, Joshua Cohen, Santhosh Kumar 
> Shanmugham, and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This diff adds an option to not retry api calls to the scheduler. For some of 
> the non-idempotent operations we would like to not automatically retry. This 
> patch makes this functionality available only to the `schedule_backup_now` 
> command.
> 
> If there is consensus, this can be added to all commands as well.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/admin/admin.py 
> 070c348d2ca5db1edecf832efd9aa5481bddaa4b 
>   src/main/python/apache/aurora/admin/aurora_admin.py 
> fbebbab8c827b5695042d18770d850e31fc38122 
>   src/main/python/apache/aurora/client/api/__init__.py 
> e1dde638bd1d686269fbcd88cb083a52e7f5dbfc 
>   src/main/python/apache/aurora/client/api/scheduler_client.py 
> 9bbfece012e48e0b1752bbefd25c89e04d312cf6 
>   src/test/python/apache/aurora/client/api/test_scheduler_client.py 
> f6018caa4f431e85a9e9ff203ac3d4b6c33f40ef 
> 
> Diff: https://reviews.apache.org/r/54957/diff/
> 
> 
> Testing
> ---
> 
> * Manuall testing
> * ./build-support/jenkins/build.sh passes
> 
> 
> Thanks,
> 
> Karthik Anantha Padmanabhan
> 
>

Re: Review Request 54957: Add option to not retry api calls to the scheduler.

[Aurora ReviewBot]

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


Ship it!




Master (38b9311) 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 Dec. 22, 2016, 12:06 a.m., Karthik Anantha Padmanabhan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54957/
> ---
> 
> (Updated Dec. 22, 2016, 12:06 a.m.)
> 
> 
> Review request for Aurora, David McLaughlin, Joshua Cohen, Santhosh Kumar 
> Shanmugham, and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This diff adds an option to not retry api calls to the scheduler. For some of 
> the non-idempotent operations we would like to not automatically retry. This 
> patch makes this functionality available only to the `schedule_backup_now` 
> command.
> 
> If there is consensus, this can be added to all commands as well.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/admin/admin.py 
> 070c348d2ca5db1edecf832efd9aa5481bddaa4b 
>   src/main/python/apache/aurora/admin/aurora_admin.py 
> fbebbab8c827b5695042d18770d850e31fc38122 
>   src/main/python/apache/aurora/client/api/__init__.py 
> e1dde638bd1d686269fbcd88cb083a52e7f5dbfc 
>   src/main/python/apache/aurora/client/api/scheduler_client.py 
> 9bbfece012e48e0b1752bbefd25c89e04d312cf6 
>   src/test/python/apache/aurora/client/api/test_scheduler_client.py 
> f6018caa4f431e85a9e9ff203ac3d4b6c33f40ef 
> 
> Diff: https://reviews.apache.org/r/54957/diff/
> 
> 
> Testing
> ---
> 
> * Manuall testing
> * ./build-support/jenkins/build.sh passes
> 
> 
> Thanks,
> 
> Karthik Anantha Padmanabhan
> 
>