Re: Review Request 27848: Add friendly error message to the client when lock is held.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27848/ --- (Updated Nov. 13, 2014, 7:32 p.m.) Review request for Aurora, Kevin Sweeney and Bill Farner. Changes --- Rebase, isort. Bugs: AURORA-885 https://issues.apache.org/jira/browse/AURORA-885 Repository: aurora Description --- Add friendly error message to the client when lock is held. Diffs (updated) - src/main/python/apache/aurora/client/api/updater_util.py 2dd44e3a30202c6359d52a3499aa09f507cc26ca src/main/python/apache/aurora/client/cli/__init__.py 83416e66a3b997e9e910d3f1c6873a0616a3f932 src/main/python/apache/aurora/client/cli/context.py a5ebbdc2ca37c5fd1813854d0a34f15511e6ca06 src/main/python/apache/aurora/client/cli/jobs.py 28f9475c5accb8c73cbc5f7a1010920479a0388e src/main/python/apache/aurora/client/cli/options.py 1f5cbb616828932742e6482867e2eca9401aad61 src/test/python/apache/aurora/client/cli/test_create.py 968c997d7ad2d26b0916ad0c2a4ac83974d5c81f src/test/python/apache/aurora/client/cli/test_kill.py 78f5f04507d7fe080a1ed5ddda692e52f66cc18d src/test/python/apache/aurora/client/cli/test_restart.py a8180a3264ac1aa2ade654985755a4dbe262dc47 src/test/python/apache/aurora/client/cli/test_supdate.py 09f6a85aebdbf0ad9c9816684f4574132205ee65 src/test/python/apache/aurora/client/cli/test_update.py a5e59e4924618ab97f18ea056ef8225e864a317d src/test/python/apache/aurora/client/cli/util.py 154fb3a7170ae81548fcbc9f3cdd6dcf9bf1942d Diff: https://reviews.apache.org/r/27848/diff/ Testing --- ./pants src/test/python/apache/aurora/client/cli/:all ./build-support/python/isort-check ./build-support/python/checkstyle-check Thanks, David McLaughlin
Re: Review Request 27848: Add friendly error message to the client when lock is held.
On Nov. 13, 2014, 4:14 a.m., Kevin Sweeney wrote: src/test/python/apache/aurora/client/cli/test_restart.py, line 16 https://reviews.apache.org/r/27848/diff/3-5/?file=757413#file757413line16 strange place for an import - should be in the 3rdparty section. did you run isort-run? isort-check said it was fine. isort-run moved the import though. - David --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27848/#review61201 --- On Nov. 13, 2014, 7:32 p.m., David McLaughlin wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27848/ --- (Updated Nov. 13, 2014, 7:32 p.m.) Review request for Aurora, Kevin Sweeney and Bill Farner. Bugs: AURORA-885 https://issues.apache.org/jira/browse/AURORA-885 Repository: aurora Description --- Add friendly error message to the client when lock is held. Diffs - src/main/python/apache/aurora/client/api/updater_util.py 2dd44e3a30202c6359d52a3499aa09f507cc26ca src/main/python/apache/aurora/client/cli/__init__.py 83416e66a3b997e9e910d3f1c6873a0616a3f932 src/main/python/apache/aurora/client/cli/context.py a5ebbdc2ca37c5fd1813854d0a34f15511e6ca06 src/main/python/apache/aurora/client/cli/jobs.py 28f9475c5accb8c73cbc5f7a1010920479a0388e src/main/python/apache/aurora/client/cli/options.py 1f5cbb616828932742e6482867e2eca9401aad61 src/test/python/apache/aurora/client/cli/test_create.py 968c997d7ad2d26b0916ad0c2a4ac83974d5c81f src/test/python/apache/aurora/client/cli/test_kill.py 78f5f04507d7fe080a1ed5ddda692e52f66cc18d src/test/python/apache/aurora/client/cli/test_restart.py a8180a3264ac1aa2ade654985755a4dbe262dc47 src/test/python/apache/aurora/client/cli/test_supdate.py 09f6a85aebdbf0ad9c9816684f4574132205ee65 src/test/python/apache/aurora/client/cli/test_update.py a5e59e4924618ab97f18ea056ef8225e864a317d src/test/python/apache/aurora/client/cli/util.py 154fb3a7170ae81548fcbc9f3cdd6dcf9bf1942d Diff: https://reviews.apache.org/r/27848/diff/ Testing --- ./pants src/test/python/apache/aurora/client/cli/:all ./build-support/python/isort-check ./build-support/python/checkstyle-check Thanks, David McLaughlin
Re: Review Request 27848: Add friendly error message to the client when lock is held.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27848/#review61304 --- @ReviewBot retry - David McLaughlin On Nov. 13, 2014, 7:32 p.m., David McLaughlin wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27848/ --- (Updated Nov. 13, 2014, 7:32 p.m.) Review request for Aurora, Kevin Sweeney and Bill Farner. Bugs: AURORA-885 https://issues.apache.org/jira/browse/AURORA-885 Repository: aurora Description --- Add friendly error message to the client when lock is held. Diffs - src/main/python/apache/aurora/client/api/updater_util.py 2dd44e3a30202c6359d52a3499aa09f507cc26ca src/main/python/apache/aurora/client/cli/__init__.py 83416e66a3b997e9e910d3f1c6873a0616a3f932 src/main/python/apache/aurora/client/cli/context.py a5ebbdc2ca37c5fd1813854d0a34f15511e6ca06 src/main/python/apache/aurora/client/cli/jobs.py 28f9475c5accb8c73cbc5f7a1010920479a0388e src/main/python/apache/aurora/client/cli/options.py 1f5cbb616828932742e6482867e2eca9401aad61 src/test/python/apache/aurora/client/cli/test_create.py 968c997d7ad2d26b0916ad0c2a4ac83974d5c81f src/test/python/apache/aurora/client/cli/test_kill.py 78f5f04507d7fe080a1ed5ddda692e52f66cc18d src/test/python/apache/aurora/client/cli/test_restart.py a8180a3264ac1aa2ade654985755a4dbe262dc47 src/test/python/apache/aurora/client/cli/test_supdate.py 09f6a85aebdbf0ad9c9816684f4574132205ee65 src/test/python/apache/aurora/client/cli/test_update.py a5e59e4924618ab97f18ea056ef8225e864a317d src/test/python/apache/aurora/client/cli/util.py 154fb3a7170ae81548fcbc9f3cdd6dcf9bf1942d Diff: https://reviews.apache.org/r/27848/diff/ Testing --- ./pants src/test/python/apache/aurora/client/cli/:all ./build-support/python/isort-check ./build-support/python/checkstyle-check Thanks, David McLaughlin
Re: Review Request 27848: Add friendly error message to the client when lock is held.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27848/#review61320 --- Ship it! Master (f8040b9) 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 Nov. 13, 2014, 9:10 p.m., David McLaughlin wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27848/ --- (Updated Nov. 13, 2014, 9:10 p.m.) Review request for Aurora, Kevin Sweeney and Bill Farner. Bugs: AURORA-885 https://issues.apache.org/jira/browse/AURORA-885 Repository: aurora Description --- Add friendly error message to the client when lock is held. Diffs - src/main/python/apache/aurora/client/api/updater_util.py 2dd44e3a30202c6359d52a3499aa09f507cc26ca src/main/python/apache/aurora/client/cli/__init__.py 83416e66a3b997e9e910d3f1c6873a0616a3f932 src/main/python/apache/aurora/client/cli/context.py a5ebbdc2ca37c5fd1813854d0a34f15511e6ca06 src/main/python/apache/aurora/client/cli/jobs.py 28f9475c5accb8c73cbc5f7a1010920479a0388e src/main/python/apache/aurora/client/cli/options.py 1f5cbb616828932742e6482867e2eca9401aad61 src/test/python/apache/aurora/client/cli/test_create.py 968c997d7ad2d26b0916ad0c2a4ac83974d5c81f src/test/python/apache/aurora/client/cli/test_kill.py 78f5f04507d7fe080a1ed5ddda692e52f66cc18d src/test/python/apache/aurora/client/cli/test_restart.py a8180a3264ac1aa2ade654985755a4dbe262dc47 src/test/python/apache/aurora/client/cli/test_supdate.py 09f6a85aebdbf0ad9c9816684f4574132205ee65 src/test/python/apache/aurora/client/cli/test_update.py a5e59e4924618ab97f18ea056ef8225e864a317d src/test/python/apache/aurora/client/cli/util.py 154fb3a7170ae81548fcbc9f3cdd6dcf9bf1942d Diff: https://reviews.apache.org/r/27848/diff/ Testing --- ./pants src/test/python/apache/aurora/client/cli/:all ./build-support/python/isort-check ./build-support/python/checkstyle-check Thanks, David McLaughlin
Re: Review Request 27848: Add friendly error message to the client when lock is held.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27848/#review61120
---
src/main/python/apache/aurora/client/cli/jobs.py
https://reviews.apache.org/r/27848/#comment102595
weird wrapping - consider wrapping the whole statement in parens so there
is one condition per line
```py
if (resp.responseCode != ResponseCode.OK
or self.wait_kill_tasks(...) != EXIT_OK):
log.error(...)
```
Also, ResponseCodes are ints - use `!=` instead of `is not`.
src/main/python/apache/aurora/client/cli/options.py
https://reviews.apache.org/r/27848/#comment102601
```py
if self.kwargs.get('dest'):
return self.kwargs['dest']
```
src/main/python/apache/aurora/client/cli/options.py
https://reviews.apache.org/r/27848/#comment102600
return self.kwargs.get('default')
src/test/python/apache/aurora/client/cli/test_create.py
https://reviews.apache.org/r/27848/#comment102603
```py
with pytest.raises(Context.CommandError):
command.execute(fake_context)
```
Otherwise this test will succeed when the command doesn't raise.
src/test/python/apache/aurora/client/cli/test_create.py
https://reviews.apache.org/r/27848/#comment102606
extract get_job_config's return value above and replace with
mock_api.assert_called_once_with(mock_job_config)
src/test/python/apache/aurora/client/cli/test_kill.py
https://reviews.apache.org/r/27848/#comment102609
mock_api.kill_job.assert_called_once_with(...) to prove the setup above
isn't a noop
src/test/python/apache/aurora/client/cli/test_kill.py
https://reviews.apache.org/r/27848/#comment102610
assert_called_once_with(...) to prove your fake object setup was needed
src/test/python/apache/aurora/client/cli/test_restart.py
https://reviews.apache.org/r/27848/#comment102611
nit: does this need to be a TestCase class? as far as I can tell you aren't
using any per-test state or setup methods so you could just define this as a
method: `def test_...` and py.test will still pick it up fine.
src/test/python/apache/aurora/client/cli/test_restart.py
https://reviews.apache.org/r/27848/#comment102612
fake_api.restart.assert_called_once_with(...)
src/test/python/apache/aurora/client/cli/test_supdate.py
https://reviews.apache.org/r/27848/#comment102614
with pytest.raises(...), see above for rationale
src/test/python/apache/aurora/client/cli/test_supdate.py
https://reviews.apache.org/r/27848/#comment102613
mock_api.start_job_update.assert_called_once_with(...)
src/test/python/apache/aurora/client/cli/test_update.py
https://reviews.apache.org/r/27848/#comment102615
pytest.raises
src/test/python/apache/aurora/client/cli/test_update.py
https://reviews.apache.org/r/27848/#comment102642
something to keep in mind here - if the return value of get_err is just the
traceback that you're throwing away above you can consider just doing this
assertion off the traceback above (and possibly removing the get_err()) method.
src/test/python/apache/aurora/client/cli/test_update.py
https://reviews.apache.org/r/27848/#comment102616
mock_api.assert_called_once_with(...)
src/test/python/apache/aurora/client/cli/util.py
https://reviews.apache.org/r/27848/#comment102643
cool
- Kevin Sweeney
On Nov. 10, 2014, 4:58 p.m., David McLaughlin wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27848/
---
(Updated Nov. 10, 2014, 4:58 p.m.)
Review request for Aurora, Kevin Sweeney and Bill Farner.
Bugs: AURORA-885
https://issues.apache.org/jira/browse/AURORA-885
Repository: aurora
Description
---
Add friendly error message to the client when lock is held.
Diffs
-
src/main/python/apache/aurora/client/cli/__init__.py
67cf40365b38b6bf395c697faf0cdb334322bdc3
src/main/python/apache/aurora/client/cli/context.py
a5ebbdc2ca37c5fd1813854d0a34f15511e6ca06
src/main/python/apache/aurora/client/cli/jobs.py
28f9475c5accb8c73cbc5f7a1010920479a0388e
src/main/python/apache/aurora/client/cli/options.py
1f5cbb616828932742e6482867e2eca9401aad61
src/test/python/apache/aurora/client/cli/test_create.py
1dec54c0da2346d4091bb3fda4508836aac0
src/test/python/apache/aurora/client/cli/test_kill.py
78f5f04507d7fe080a1ed5ddda692e52f66cc18d
src/test/python/apache/aurora/client/cli/test_restart.py
a8180a3264ac1aa2ade654985755a4dbe262dc47
src/test/python/apache/aurora/client/cli/test_supdate.py
09f6a85aebdbf0ad9c9816684f4574132205ee65
src/test/python/apache/aurora/client/cli/test_update.py
a5e59e4924618ab97f18ea056ef8225e864a317d
Re: Review Request 27848: Add friendly error message to the client when lock is held.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27848/ --- (Updated Nov. 13, 2014, 1:48 a.m.) Review request for Aurora, Kevin Sweeney and Bill Farner. Changes --- Forgot to run checkstyle-check. Bugs: AURORA-885 https://issues.apache.org/jira/browse/AURORA-885 Repository: aurora Description --- Add friendly error message to the client when lock is held. Diffs (updated) - src/main/python/apache/aurora/client/api/updater_util.py 2dd44e3a30202c6359d52a3499aa09f507cc26ca src/main/python/apache/aurora/client/cli/__init__.py 67cf40365b38b6bf395c697faf0cdb334322bdc3 src/main/python/apache/aurora/client/cli/context.py a5ebbdc2ca37c5fd1813854d0a34f15511e6ca06 src/main/python/apache/aurora/client/cli/jobs.py 28f9475c5accb8c73cbc5f7a1010920479a0388e src/main/python/apache/aurora/client/cli/options.py 1f5cbb616828932742e6482867e2eca9401aad61 src/test/python/apache/aurora/client/cli/test_create.py 1dec54c0da2346d4091bb3fda4508836aac0 src/test/python/apache/aurora/client/cli/test_kill.py 78f5f04507d7fe080a1ed5ddda692e52f66cc18d src/test/python/apache/aurora/client/cli/test_restart.py a8180a3264ac1aa2ade654985755a4dbe262dc47 src/test/python/apache/aurora/client/cli/test_supdate.py 09f6a85aebdbf0ad9c9816684f4574132205ee65 src/test/python/apache/aurora/client/cli/test_update.py a5e59e4924618ab97f18ea056ef8225e864a317d src/test/python/apache/aurora/client/cli/util.py 154fb3a7170ae81548fcbc9f3cdd6dcf9bf1942d Diff: https://reviews.apache.org/r/27848/diff/ Testing --- ./pants src/test/python/apache/aurora/client/cli/:all ./build-support/python/isort-check ./build-support/python/checkstyle-check Thanks, David McLaughlin
Re: Review Request 27848: Add friendly error message to the client when lock is held.
On Nov. 13, 2014, 12:36 a.m., Kevin Sweeney wrote:
src/main/python/apache/aurora/client/cli/jobs.py, line 331
https://reviews.apache.org/r/27848/diff/3/?file=757409#file757409line331
weird wrapping - consider wrapping the whole statement in parens so
there is one condition per line
```py
if (resp.responseCode != ResponseCode.OK
or self.wait_kill_tasks(...) != EXIT_OK):
log.error(...)
```
Also, ResponseCodes are ints - use `!=` instead of `is not`.
Done.
On Nov. 13, 2014, 12:36 a.m., Kevin Sweeney wrote:
src/main/python/apache/aurora/client/cli/options.py, line 63
https://reviews.apache.org/r/27848/diff/3/?file=757410#file757410line63
```py
if self.kwargs.get('dest'):
return self.kwargs['dest']
```
Fixed.
On Nov. 13, 2014, 12:36 a.m., Kevin Sweeney wrote:
src/main/python/apache/aurora/client/cli/options.py, lines 70-72
https://reviews.apache.org/r/27848/diff/3/?file=757410#file757410line70
return self.kwargs.get('default')
Fixed.
On Nov. 13, 2014, 12:36 a.m., Kevin Sweeney wrote:
src/test/python/apache/aurora/client/cli/test_create.py, lines 74-77
https://reviews.apache.org/r/27848/diff/3/?file=757411#file757411line74
```py
with pytest.raises(Context.CommandError):
command.execute(fake_context)
```
Otherwise this test will succeed when the command doesn't raise.
Well the assertion still fails if the message isn't added to the output. This
suggestion is better than the try except anyway. Done.
On Nov. 13, 2014, 12:36 a.m., Kevin Sweeney wrote:
src/test/python/apache/aurora/client/cli/test_create.py, line 79
https://reviews.apache.org/r/27848/diff/3/?file=757411#file757411line79
extract get_job_config's return value above and replace with
mock_api.assert_called_once_with(mock_job_config)
Done.
On Nov. 13, 2014, 12:36 a.m., Kevin Sweeney wrote:
src/test/python/apache/aurora/client/cli/test_kill.py, line 75
https://reviews.apache.org/r/27848/diff/3/?file=757412#file757412line75
mock_api.kill_job.assert_called_once_with(...) to prove the setup above
isn't a noop
Done.
On Nov. 13, 2014, 12:36 a.m., Kevin Sweeney wrote:
src/test/python/apache/aurora/client/cli/test_kill.py, line 103
https://reviews.apache.org/r/27848/diff/3/?file=757412#file757412line103
assert_called_once_with(...) to prove your fake object setup was needed
Done.
On Nov. 13, 2014, 12:36 a.m., Kevin Sweeney wrote:
src/test/python/apache/aurora/client/cli/test_restart.py, line 33
https://reviews.apache.org/r/27848/diff/3/?file=757413#file757413line33
nit: does this need to be a TestCase class? as far as I can tell you
aren't using any per-test state or setup methods so you could just define
this as a method: `def test_...` and py.test will still pick it up fine.
The idea is that we'd eventually get full coverage of these commands using this
style. In this case I'm still only using classes for grouping while they live
next to the existing tests. I could split the new style tests into their own
files and just use functions? But I sort of wanted them to be next to the
existing integrationy tests so people cargo cult the correct thing when adding
more tests. Thoughts?
On Nov. 13, 2014, 12:36 a.m., Kevin Sweeney wrote:
src/test/python/apache/aurora/client/cli/test_restart.py, line 54
https://reviews.apache.org/r/27848/diff/3/?file=757413#file757413line54
fake_api.restart.assert_called_once_with(...)
Done.
On Nov. 13, 2014, 12:36 a.m., Kevin Sweeney wrote:
src/test/python/apache/aurora/client/cli/test_supdate.py, lines 67-70
https://reviews.apache.org/r/27848/diff/3/?file=757414#file757414line67
with pytest.raises(...), see above for rationale
Done.
On Nov. 13, 2014, 12:36 a.m., Kevin Sweeney wrote:
src/test/python/apache/aurora/client/cli/test_supdate.py, line 73
https://reviews.apache.org/r/27848/diff/3/?file=757414#file757414line73
mock_api.start_job_update.assert_called_once_with(...)
Done.
On Nov. 13, 2014, 12:36 a.m., Kevin Sweeney wrote:
src/test/python/apache/aurora/client/cli/test_update.py, line 76
https://reviews.apache.org/r/27848/diff/3/?file=757415#file757415line76
something to keep in mind here - if the return value of get_err is just
the traceback that you're throwing away above you can consider just doing
this assertion off the traceback above (and possibly removing the
get_err()) method.
It's not just the traceback, the message comes from check_and_log_response. I'm
not a huge fan of this design (or the context heirarchy in general...) but I
don't have enough context yet to go in and tear it up in favour of just
throwing exceptions (AFAICT the main hurdle is that the current design allows
for multiple messages of different 'types' to be propagated to the user,
including messages from
Re: Review Request 27848: Add friendly error message to the client when lock is held.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27848/#review61175 --- This patch does not apply cleanly on master (6950d50), do you need to rebase? I will refresh this build result if you post a review containing @ReviewBot retry - Aurora ReviewBot On Nov. 13, 2014, 1:48 a.m., David McLaughlin wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27848/ --- (Updated Nov. 13, 2014, 1:48 a.m.) Review request for Aurora, Kevin Sweeney and Bill Farner. Bugs: AURORA-885 https://issues.apache.org/jira/browse/AURORA-885 Repository: aurora Description --- Add friendly error message to the client when lock is held. Diffs - src/main/python/apache/aurora/client/api/updater_util.py 2dd44e3a30202c6359d52a3499aa09f507cc26ca src/main/python/apache/aurora/client/cli/__init__.py 67cf40365b38b6bf395c697faf0cdb334322bdc3 src/main/python/apache/aurora/client/cli/context.py a5ebbdc2ca37c5fd1813854d0a34f15511e6ca06 src/main/python/apache/aurora/client/cli/jobs.py 28f9475c5accb8c73cbc5f7a1010920479a0388e src/main/python/apache/aurora/client/cli/options.py 1f5cbb616828932742e6482867e2eca9401aad61 src/test/python/apache/aurora/client/cli/test_create.py 1dec54c0da2346d4091bb3fda4508836aac0 src/test/python/apache/aurora/client/cli/test_kill.py 78f5f04507d7fe080a1ed5ddda692e52f66cc18d src/test/python/apache/aurora/client/cli/test_restart.py a8180a3264ac1aa2ade654985755a4dbe262dc47 src/test/python/apache/aurora/client/cli/test_supdate.py 09f6a85aebdbf0ad9c9816684f4574132205ee65 src/test/python/apache/aurora/client/cli/test_update.py a5e59e4924618ab97f18ea056ef8225e864a317d src/test/python/apache/aurora/client/cli/util.py 154fb3a7170ae81548fcbc9f3cdd6dcf9bf1942d Diff: https://reviews.apache.org/r/27848/diff/ Testing --- ./pants src/test/python/apache/aurora/client/cli/:all ./build-support/python/isort-check ./build-support/python/checkstyle-check Thanks, David McLaughlin
Re: Review Request 27848: Add friendly error message to the client when lock is held.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27848/ --- (Updated Nov. 13, 2014, 1:45 a.m.) Review request for Aurora, Kevin Sweeney and Bill Farner. Changes --- feedback. Bugs: AURORA-885 https://issues.apache.org/jira/browse/AURORA-885 Repository: aurora Description --- Add friendly error message to the client when lock is held. Diffs (updated) - src/main/python/apache/aurora/client/api/updater_util.py 2dd44e3a30202c6359d52a3499aa09f507cc26ca src/main/python/apache/aurora/client/cli/__init__.py 67cf40365b38b6bf395c697faf0cdb334322bdc3 src/main/python/apache/aurora/client/cli/context.py a5ebbdc2ca37c5fd1813854d0a34f15511e6ca06 src/main/python/apache/aurora/client/cli/jobs.py 28f9475c5accb8c73cbc5f7a1010920479a0388e src/main/python/apache/aurora/client/cli/options.py 1f5cbb616828932742e6482867e2eca9401aad61 src/test/python/apache/aurora/client/cli/test_create.py 1dec54c0da2346d4091bb3fda4508836aac0 src/test/python/apache/aurora/client/cli/test_kill.py 78f5f04507d7fe080a1ed5ddda692e52f66cc18d src/test/python/apache/aurora/client/cli/test_restart.py a8180a3264ac1aa2ade654985755a4dbe262dc47 src/test/python/apache/aurora/client/cli/test_supdate.py 09f6a85aebdbf0ad9c9816684f4574132205ee65 src/test/python/apache/aurora/client/cli/test_update.py a5e59e4924618ab97f18ea056ef8225e864a317d src/test/python/apache/aurora/client/cli/util.py 154fb3a7170ae81548fcbc9f3cdd6dcf9bf1942d Diff: https://reviews.apache.org/r/27848/diff/ Testing --- ./pants src/test/python/apache/aurora/client/cli/:all ./build-support/python/isort-check ./build-support/python/checkstyle-check Thanks, David McLaughlin
Re: Review Request 27848: Add friendly error message to the client when lock is held.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27848/#review61201 --- Ship it! lgtm once bot agrees, please follow-up with a ticket for fixing the import sorter if the current patch passed it src/test/python/apache/aurora/client/cli/test_kill.py https://reviews.apache.org/r/27848/#comment102722 this seems like a strange place to place this import (but lgtm if the tool says it's fine) src/test/python/apache/aurora/client/cli/test_restart.py https://reviews.apache.org/r/27848/#comment102723 strange place for an import - should be in the 3rdparty section. did you run isort-run? - Kevin Sweeney On Nov. 12, 2014, 5:48 p.m., David McLaughlin wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27848/ --- (Updated Nov. 12, 2014, 5:48 p.m.) Review request for Aurora, Kevin Sweeney and Bill Farner. Bugs: AURORA-885 https://issues.apache.org/jira/browse/AURORA-885 Repository: aurora Description --- Add friendly error message to the client when lock is held. Diffs - src/main/python/apache/aurora/client/api/updater_util.py 2dd44e3a30202c6359d52a3499aa09f507cc26ca src/main/python/apache/aurora/client/cli/__init__.py 67cf40365b38b6bf395c697faf0cdb334322bdc3 src/main/python/apache/aurora/client/cli/context.py a5ebbdc2ca37c5fd1813854d0a34f15511e6ca06 src/main/python/apache/aurora/client/cli/jobs.py 28f9475c5accb8c73cbc5f7a1010920479a0388e src/main/python/apache/aurora/client/cli/options.py 1f5cbb616828932742e6482867e2eca9401aad61 src/test/python/apache/aurora/client/cli/test_create.py 1dec54c0da2346d4091bb3fda4508836aac0 src/test/python/apache/aurora/client/cli/test_kill.py 78f5f04507d7fe080a1ed5ddda692e52f66cc18d src/test/python/apache/aurora/client/cli/test_restart.py a8180a3264ac1aa2ade654985755a4dbe262dc47 src/test/python/apache/aurora/client/cli/test_supdate.py 09f6a85aebdbf0ad9c9816684f4574132205ee65 src/test/python/apache/aurora/client/cli/test_update.py a5e59e4924618ab97f18ea056ef8225e864a317d src/test/python/apache/aurora/client/cli/util.py 154fb3a7170ae81548fcbc9f3cdd6dcf9bf1942d Diff: https://reviews.apache.org/r/27848/diff/ Testing --- ./pants src/test/python/apache/aurora/client/cli/:all ./build-support/python/isort-check ./build-support/python/checkstyle-check Thanks, David McLaughlin
Re: Review Request 27848: Add friendly error message to the client when lock is held.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27848/#review60892 --- Ship it! src/main/python/apache/aurora/client/cli/context.py https://reviews.apache.org/r/27848/#comment102299 Is this going to produce some redundant output? - Bill Farner On Nov. 11, 2014, 12:58 a.m., David McLaughlin wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27848/ --- (Updated Nov. 11, 2014, 12:58 a.m.) Review request for Aurora, Kevin Sweeney and Bill Farner. Bugs: AURORA-885 https://issues.apache.org/jira/browse/AURORA-885 Repository: aurora Description --- Add friendly error message to the client when lock is held. Diffs - src/main/python/apache/aurora/client/cli/__init__.py 67cf40365b38b6bf395c697faf0cdb334322bdc3 src/main/python/apache/aurora/client/cli/context.py a5ebbdc2ca37c5fd1813854d0a34f15511e6ca06 src/main/python/apache/aurora/client/cli/jobs.py 28f9475c5accb8c73cbc5f7a1010920479a0388e src/main/python/apache/aurora/client/cli/options.py 1f5cbb616828932742e6482867e2eca9401aad61 src/test/python/apache/aurora/client/cli/test_create.py 1dec54c0da2346d4091bb3fda4508836aac0 src/test/python/apache/aurora/client/cli/test_kill.py 78f5f04507d7fe080a1ed5ddda692e52f66cc18d src/test/python/apache/aurora/client/cli/test_restart.py a8180a3264ac1aa2ade654985755a4dbe262dc47 src/test/python/apache/aurora/client/cli/test_supdate.py 09f6a85aebdbf0ad9c9816684f4574132205ee65 src/test/python/apache/aurora/client/cli/test_update.py a5e59e4924618ab97f18ea056ef8225e864a317d src/test/python/apache/aurora/client/cli/util.py 154fb3a7170ae81548fcbc9f3cdd6dcf9bf1942d Diff: https://reviews.apache.org/r/27848/diff/ Testing --- ./pants src/test/python/apache/aurora/client/cli/:all ./build-support/python/isort-check ./build-support/python/checkstyle-check Thanks, David McLaughlin
Re: Review Request 27848: Add friendly error message to the client when lock is held.
On Nov. 12, 2014, 1:28 a.m., Bill Farner wrote: src/main/python/apache/aurora/client/cli/context.py, line 152 https://reviews.apache.org/r/27848/diff/3/?file=757408#file757408line152 Is this going to produce some redundant output? See reply below, not sure why it didn't reply inline. - David --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27848/#review60892 --- On Nov. 11, 2014, 12:58 a.m., David McLaughlin wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27848/ --- (Updated Nov. 11, 2014, 12:58 a.m.) Review request for Aurora, Kevin Sweeney and Bill Farner. Bugs: AURORA-885 https://issues.apache.org/jira/browse/AURORA-885 Repository: aurora Description --- Add friendly error message to the client when lock is held. Diffs - src/main/python/apache/aurora/client/cli/__init__.py 67cf40365b38b6bf395c697faf0cdb334322bdc3 src/main/python/apache/aurora/client/cli/context.py a5ebbdc2ca37c5fd1813854d0a34f15511e6ca06 src/main/python/apache/aurora/client/cli/jobs.py 28f9475c5accb8c73cbc5f7a1010920479a0388e src/main/python/apache/aurora/client/cli/options.py 1f5cbb616828932742e6482867e2eca9401aad61 src/test/python/apache/aurora/client/cli/test_create.py 1dec54c0da2346d4091bb3fda4508836aac0 src/test/python/apache/aurora/client/cli/test_kill.py 78f5f04507d7fe080a1ed5ddda692e52f66cc18d src/test/python/apache/aurora/client/cli/test_restart.py a8180a3264ac1aa2ade654985755a4dbe262dc47 src/test/python/apache/aurora/client/cli/test_supdate.py 09f6a85aebdbf0ad9c9816684f4574132205ee65 src/test/python/apache/aurora/client/cli/test_update.py a5e59e4924618ab97f18ea056ef8225e864a317d src/test/python/apache/aurora/client/cli/util.py 154fb3a7170ae81548fcbc9f3cdd6dcf9bf1942d Diff: https://reviews.apache.org/r/27848/diff/ Testing --- ./pants src/test/python/apache/aurora/client/cli/:all ./build-support/python/isort-check ./build-support/python/checkstyle-check Thanks, David McLaughlin
Review Request 27848: Add friendly error message to the client when lock is held.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27848/ --- Review request for Aurora, Kevin Sweeney and Bill Farner. Bugs: AURORA-885 https://issues.apache.org/jira/browse/AURORA-885 Repository: aurora Description --- Add friendly error message to the client when lock is held. Diffs - src/main/python/apache/aurora/client/cli/__init__.py 67cf40365b38b6bf395c697faf0cdb334322bdc3 src/main/python/apache/aurora/client/cli/context.py a5ebbdc2ca37c5fd1813854d0a34f15511e6ca06 src/main/python/apache/aurora/client/cli/jobs.py 28f9475c5accb8c73cbc5f7a1010920479a0388e src/main/python/apache/aurora/client/cli/options.py 1f5cbb616828932742e6482867e2eca9401aad61 src/test/python/apache/aurora/client/cli/test_create.py 1dec54c0da2346d4091bb3fda4508836aac0 src/test/python/apache/aurora/client/cli/test_kill.py 78f5f04507d7fe080a1ed5ddda692e52f66cc18d src/test/python/apache/aurora/client/cli/test_restart.py a8180a3264ac1aa2ade654985755a4dbe262dc47 src/test/python/apache/aurora/client/cli/test_supdate.py 09f6a85aebdbf0ad9c9816684f4574132205ee65 src/test/python/apache/aurora/client/cli/test_update.py a5e59e4924618ab97f18ea056ef8225e864a317d src/test/python/apache/aurora/client/cli/util.py 154fb3a7170ae81548fcbc9f3cdd6dcf9bf1942d Diff: https://reviews.apache.org/r/27848/diff/ Testing --- ./pants src/test/python/apache/aurora/client/cli/:all ./build-support/python/isort-check ./build-support/python/checkstyle-check Thanks, David McLaughlin
Re: Review Request 27848: Add friendly error message to the client when lock is held.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27848/ --- (Updated Nov. 11, 2014, 12:29 a.m.) Review request for Aurora, Kevin Sweeney and Bill Farner. Changes --- Removed stray debug statement. Bugs: AURORA-885 https://issues.apache.org/jira/browse/AURORA-885 Repository: aurora Description --- Add friendly error message to the client when lock is held. Diffs (updated) - src/main/python/apache/aurora/client/cli/__init__.py 67cf40365b38b6bf395c697faf0cdb334322bdc3 src/main/python/apache/aurora/client/cli/context.py a5ebbdc2ca37c5fd1813854d0a34f15511e6ca06 src/main/python/apache/aurora/client/cli/jobs.py 28f9475c5accb8c73cbc5f7a1010920479a0388e src/main/python/apache/aurora/client/cli/options.py 1f5cbb616828932742e6482867e2eca9401aad61 src/test/python/apache/aurora/client/cli/test_create.py 1dec54c0da2346d4091bb3fda4508836aac0 src/test/python/apache/aurora/client/cli/test_kill.py 78f5f04507d7fe080a1ed5ddda692e52f66cc18d src/test/python/apache/aurora/client/cli/test_restart.py a8180a3264ac1aa2ade654985755a4dbe262dc47 src/test/python/apache/aurora/client/cli/test_supdate.py 09f6a85aebdbf0ad9c9816684f4574132205ee65 src/test/python/apache/aurora/client/cli/test_update.py a5e59e4924618ab97f18ea056ef8225e864a317d src/test/python/apache/aurora/client/cli/util.py 154fb3a7170ae81548fcbc9f3cdd6dcf9bf1942d Diff: https://reviews.apache.org/r/27848/diff/ Testing --- ./pants src/test/python/apache/aurora/client/cli/:all ./build-support/python/isort-check ./build-support/python/checkstyle-check Thanks, David McLaughlin
Re: Review Request 27848: Add friendly error message to the client when lock is held.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27848/#review60723 --- Master (01958b1) is red with this patch. ./build-support/jenkins/build.sh Skipping installation of /home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter/__init__.py (namespace package) Skipping installation of /home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter/common/__init__.py (namespace package) Installing /home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter.common.string-0.3.0-py2.7-nspkg.pth Running setup.py install for twitter.common.options Skipping installation of /home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter/__init__.py (namespace package) Skipping installation of /home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter/common/__init__.py (namespace package) Installing /home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter.common.options-0.3.0-py2.7-nspkg.pth Running setup.py install for twitter.common.dirutil Skipping installation of /home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter/__init__.py (namespace package) Skipping installation of /home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter/common/__init__.py (namespace package) Installing /home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter.common.dirutil-0.3.0-py2.7-nspkg.pth Running setup.py install for twitter.common.contextutil Skipping installation of /home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter/__init__.py (namespace package) Skipping installation of /home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter/common/__init__.py (namespace package) Installing /home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter.common.contextutil-0.3.0-py2.7-nspkg.pth Running setup.py install for twitter.common.lang Skipping installation of /home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter/__init__.py (namespace package) Skipping installation of /home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter/common/__init__.py (namespace package) Installing /home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter.common.lang-0.3.0-py2.7-nspkg.pth Successfully installed twitter.checkstyle pyflakes pep8 GitPython twitter.common.app gitdb twitter.common.process twitter.common.log twitter.common.util twitter.common.collections async smmap twitter.common.string twitter.common.options twitter.common.dirutil twitter.common.contextutil twitter.common.lang Cleaning up... E303:ERROR src/test/python/apache/aurora/client/cli/test_restart.py:058 too many blank lines (3) |class TestRestartCommand(AuroraClientCommandTest): T302:ERROR src/test/python/apache/aurora/client/cli/test_restart.py:058 Expected 2 blank lines, found 3 |class TestRestartCommand(AuroraClientCommandTest): E303:ERROR src/test/python/apache/aurora/client/cli/test_supdate.py:076 too many blank lines (3) |class TestUpdateCommand(AuroraClientCommandTest): T302:ERROR src/test/python/apache/aurora/client/cli/test_supdate.py:076 Expected 2 blank lines, found 3 |class TestUpdateCommand(AuroraClientCommandTest): E303:ERROR src/test/python/apache/aurora/client/cli/test_kill.py:108 too many blank lines (3) |class TestClientKillCommand(AuroraClientCommandTest): T302:ERROR src/test/python/apache/aurora/client/cli/test_kill.py:108 Expected 2 blank lines, found 3 |class TestClientKillCommand(AuroraClientCommandTest): E303:ERROR src/test/python/apache/aurora/client/cli/test_update.py:080 too many blank lines (3) |class TestUpdateCommand(AuroraClientCommandTest): T302:ERROR src/test/python/apache/aurora/client/cli/test_update.py:080 Expected 2 blank lines, found 3 |class TestUpdateCommand(AuroraClientCommandTest): - Aurora ReviewBot On Nov. 11, 2014, 12:29 a.m., David McLaughlin wrote:
Re: Review Request 27848: Add friendly error message to the client when lock is held.
On Nov. 11, 2014, 12:43 a.m., Aurora ReviewBot wrote: Master (01958b1) is red with this patch. ./build-support/jenkins/build.sh Skipping installation of /home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter/__init__.py (namespace package) Skipping installation of /home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter/common/__init__.py (namespace package) Installing /home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter.common.string-0.3.0-py2.7-nspkg.pth Running setup.py install for twitter.common.options Skipping installation of /home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter/__init__.py (namespace package) Skipping installation of /home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter/common/__init__.py (namespace package) Installing /home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter.common.options-0.3.0-py2.7-nspkg.pth Running setup.py install for twitter.common.dirutil Skipping installation of /home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter/__init__.py (namespace package) Skipping installation of /home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter/common/__init__.py (namespace package) Installing /home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter.common.dirutil-0.3.0-py2.7-nspkg.pth Running setup.py install for twitter.common.contextutil Skipping installation of /home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter/__init__.py (namespace package) Skipping installation of /home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter/common/__init__.py (namespace package) Installing /home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter.common.contextutil-0.3.0-py2.7-nspkg.pth Running setup.py install for twitter.common.lang Skipping installation of /home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter/__init__.py (namespace package) Skipping installation of /home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter/common/__init__.py (namespace package) Installing /home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/python/checkstyle.venv/lib/python2.7/site-packages/twitter.common.lang-0.3.0-py2.7-nspkg.pth Successfully installed twitter.checkstyle pyflakes pep8 GitPython twitter.common.app gitdb twitter.common.process twitter.common.log twitter.common.util twitter.common.collections async smmap twitter.common.string twitter.common.options twitter.common.dirutil twitter.common.contextutil twitter.common.lang Cleaning up... E303:ERROR src/test/python/apache/aurora/client/cli/test_restart.py:058 too many blank lines (3) |class TestRestartCommand(AuroraClientCommandTest): T302:ERROR src/test/python/apache/aurora/client/cli/test_restart.py:058 Expected 2 blank lines, found 3 |class TestRestartCommand(AuroraClientCommandTest): E303:ERROR src/test/python/apache/aurora/client/cli/test_supdate.py:076 too many blank lines (3) |class TestUpdateCommand(AuroraClientCommandTest): T302:ERROR src/test/python/apache/aurora/client/cli/test_supdate.py:076 Expected 2 blank lines, found 3 |class TestUpdateCommand(AuroraClientCommandTest): E303:ERROR src/test/python/apache/aurora/client/cli/test_kill.py:108 too many blank lines (3) |class TestClientKillCommand(AuroraClientCommandTest): T302:ERROR src/test/python/apache/aurora/client/cli/test_kill.py:108 Expected 2 blank lines, found 3 |class TestClientKillCommand(AuroraClientCommandTest): E303:ERROR src/test/python/apache/aurora/client/cli/test_update.py:080 too many blank lines (3) |class TestUpdateCommand(AuroraClientCommandTest): T302:ERROR src/test/python/apache/aurora/client/cli/test_update.py:080 Expected 2 blank lines, found 3 |class TestUpdateCommand(AuroraClientCommandTest): Strange: $ ./build-support/python/checkstyle-check tw-172-25-139-176:incubator-aurora
Re: Review Request 27848: Add friendly error message to the client when lock is held.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27848/#review60728 --- Ship it! Master (01958b1) is green with this patch. ./build-support/jenkins/build.sh - Aurora ReviewBot On Nov. 11, 2014, 12:58 a.m., David McLaughlin wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27848/ --- (Updated Nov. 11, 2014, 12:58 a.m.) Review request for Aurora, Kevin Sweeney and Bill Farner. Bugs: AURORA-885 https://issues.apache.org/jira/browse/AURORA-885 Repository: aurora Description --- Add friendly error message to the client when lock is held. Diffs - src/main/python/apache/aurora/client/cli/__init__.py 67cf40365b38b6bf395c697faf0cdb334322bdc3 src/main/python/apache/aurora/client/cli/context.py a5ebbdc2ca37c5fd1813854d0a34f15511e6ca06 src/main/python/apache/aurora/client/cli/jobs.py 28f9475c5accb8c73cbc5f7a1010920479a0388e src/main/python/apache/aurora/client/cli/options.py 1f5cbb616828932742e6482867e2eca9401aad61 src/test/python/apache/aurora/client/cli/test_create.py 1dec54c0da2346d4091bb3fda4508836aac0 src/test/python/apache/aurora/client/cli/test_kill.py 78f5f04507d7fe080a1ed5ddda692e52f66cc18d src/test/python/apache/aurora/client/cli/test_restart.py a8180a3264ac1aa2ade654985755a4dbe262dc47 src/test/python/apache/aurora/client/cli/test_supdate.py 09f6a85aebdbf0ad9c9816684f4574132205ee65 src/test/python/apache/aurora/client/cli/test_update.py a5e59e4924618ab97f18ea056ef8225e864a317d src/test/python/apache/aurora/client/cli/util.py 154fb3a7170ae81548fcbc9f3cdd6dcf9bf1942d Diff: https://reviews.apache.org/r/27848/diff/ Testing --- ./pants src/test/python/apache/aurora/client/cli/:all ./build-support/python/isort-check ./build-support/python/checkstyle-check Thanks, David McLaughlin
