You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@aurora.apache.org by Joe Smith <ya...@gmail.com> on 2014/10/16 08:02:10 UTC
Review Request 26802: Set a default for the error log dir
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26802/
-----------------------------------------------------------
Review request for Aurora, Maxim Khutornenko, Bill Farner, and Zameer Manji.
Bugs: AURORA-827
https://issues.apache.org/jira/browse/AURORA-827
Repository: aurora
Description
-------
Set a default for the error log dir
This has been a weird issue to wrap my head around, it's really using lots of low-level systems (writing tracebacks to files) so I feel like I've done some weird things in the tests. Feedback and critque welcome + appreciated.
Diffs
-----
src/main/python/apache/aurora/client/cli/__init__.py da9d5b6ba4d22ba1f444341b97bbcfaf7889a4a8
src/test/python/apache/aurora/client/cli/BUILD d33e86643a59879c115876c98bd1dc19aa7ae61c
src/test/python/apache/aurora/client/cli/test_aurora_command_line.py PRE-CREATION
Diff: https://reviews.apache.org/r/26802/diff/
Testing
-------
[tw-172-25-132-201 aurora (yasumoto/error_log_dir_default)]$ ./pants src/test/python/apache/aurora/client/cli:aurora_command_line
Build operating on top level addresses: set([BuildFileAddress(/Users/jsmith/workspace/aurora/src/test/python/apache/aurora/client/cli/BUILD, aurora_command_line)])
==================================================================== test session starts =====================================================================
platform darwin -- Python 2.7.5 -- py-1.4.25 -- pytest-2.6.3
plugins: cov, timeout
collected 1 items
src/test/python/apache/aurora/client/cli/test_aurora_command_line.py .
================================================================== 1 passed in 0.63 seconds ==================================================================
src.test.python.apache.aurora.client.cli.aurora_command_line ..... SUCCESS
Thanks,
Joe Smith
Re: Review Request 26802: Set a default for the error log dir
Posted by Joe Smith <ya...@gmail.com>.
> On Oct. 16, 2014, 10 a.m., Bill Farner wrote:
> > src/main/python/apache/aurora/client/cli/__init__.py, line 114
> > <https://reviews.apache.org/r/26802/diff/1/?file=722890#file722890line114>
> >
> > Can you try to remove noqa, and if it is suppressing an error, leave a comment indicating why the pyflakes errors are suppressed?
Yep, these should've been classmethods.
> On Oct. 16, 2014, 10 a.m., Bill Farner wrote:
> > src/main/python/apache/aurora/client/cli/__init__.py, line 118
> > <https://reviews.apache.org/r/26802/diff/1/?file=722890#file722890line118>
> >
> > Ditto
> >
> >
> > also, more concise:
> >
> > return Context.ERROR_LOG_DIR or '.'
Done.
> On Oct. 16, 2014, 10 a.m., Bill Farner wrote:
> > src/test/python/apache/aurora/client/cli/BUILD, line 140
> > <https://reviews.apache.org/r/26802/diff/1/?file=722891#file722891line140>
> >
> > remove extra newline
Done.
> On Oct. 16, 2014, 10 a.m., Bill Farner wrote:
> > src/test/python/apache/aurora/client/cli/test_aurora_command_line.py, line 34
> > <https://reviews.apache.org/r/26802/diff/1/?file=722892#file722892line34>
> >
> > I'm not sure what the right answer is here, and we may be too far down the monkeypatch road to change tacks in this effort, but overriding builtins feels like a special kind of abstraction violation.
> >
> > Do any other reviewers know the lay of the land well enough to suggest a better way? Ideally we would be injecting this functionallity rather than patching it, but i don't want to turn a small fix into a giant yak shave.
>
> Maxim Khutornenko wrote:
> Seems like the appropriate use of patching given the current implementation. Refactoring here would only hurt readability and would rather feel unnatural.
>
> Joshua Cohen wrote:
> +1, I'm not an expert on what is or isn't pythonic, but this certainly seems in line with how I've done testing in other dynamic languages. I'm not sure cluttering up the interface with explicit injection points for dependencies is warranted/necessary when you can just patch them in tests?
Unless we switch to using interfaces and injection, patching is indeed a ~recommended approach.
- Joe
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26802/#review56929
-----------------------------------------------------------
On Oct. 15, 2014, 11:02 p.m., Joe Smith wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26802/
> -----------------------------------------------------------
>
> (Updated Oct. 15, 2014, 11:02 p.m.)
>
>
> Review request for Aurora, Maxim Khutornenko, Bill Farner, and Zameer Manji.
>
>
> Bugs: AURORA-827
> https://issues.apache.org/jira/browse/AURORA-827
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Set a default for the error log dir
>
> This has been a weird issue to wrap my head around, it's really using lots of low-level systems (writing tracebacks to files) so I feel like I've done some weird things in the tests. Feedback and critque welcome + appreciated.
>
>
> Diffs
> -----
>
> src/main/python/apache/aurora/client/cli/__init__.py da9d5b6ba4d22ba1f444341b97bbcfaf7889a4a8
> src/test/python/apache/aurora/client/cli/BUILD d33e86643a59879c115876c98bd1dc19aa7ae61c
> src/test/python/apache/aurora/client/cli/test_aurora_command_line.py PRE-CREATION
>
> Diff: https://reviews.apache.org/r/26802/diff/
>
>
> Testing
> -------
>
> [tw-172-25-132-201 aurora (yasumoto/error_log_dir_default)]$ ./pants src/test/python/apache/aurora/client/cli:aurora_command_line
> Build operating on top level addresses: set([BuildFileAddress(/Users/jsmith/workspace/aurora/src/test/python/apache/aurora/client/cli/BUILD, aurora_command_line)])
> ==================================================================== test session starts =====================================================================
> platform darwin -- Python 2.7.5 -- py-1.4.25 -- pytest-2.6.3
> plugins: cov, timeout
> collected 1 items
>
> src/test/python/apache/aurora/client/cli/test_aurora_command_line.py .
>
> ================================================================== 1 passed in 0.63 seconds ==================================================================
> src.test.python.apache.aurora.client.cli.aurora_command_line ..... SUCCESS
>
>
> Thanks,
>
> Joe Smith
>
>
Re: Review Request 26802: Set a default for the error log dir
Posted by Joshua Cohen <jc...@twopensource.com>.
> On Oct. 16, 2014, 5 p.m., Bill Farner wrote:
> > src/test/python/apache/aurora/client/cli/test_aurora_command_line.py, line 34
> > <https://reviews.apache.org/r/26802/diff/1/?file=722892#file722892line34>
> >
> > I'm not sure what the right answer is here, and we may be too far down the monkeypatch road to change tacks in this effort, but overriding builtins feels like a special kind of abstraction violation.
> >
> > Do any other reviewers know the lay of the land well enough to suggest a better way? Ideally we would be injecting this functionallity rather than patching it, but i don't want to turn a small fix into a giant yak shave.
>
> Maxim Khutornenko wrote:
> Seems like the appropriate use of patching given the current implementation. Refactoring here would only hurt readability and would rather feel unnatural.
+1, I'm not an expert on what is or isn't pythonic, but this certainly seems in line with how I've done testing in other dynamic languages. I'm not sure cluttering up the interface with explicit injection points for dependencies is warranted/necessary when you can just patch them in tests?
- Joshua
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26802/#review56929
-----------------------------------------------------------
On Oct. 16, 2014, 6:02 a.m., Joe Smith wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26802/
> -----------------------------------------------------------
>
> (Updated Oct. 16, 2014, 6:02 a.m.)
>
>
> Review request for Aurora, Maxim Khutornenko, Bill Farner, and Zameer Manji.
>
>
> Bugs: AURORA-827
> https://issues.apache.org/jira/browse/AURORA-827
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Set a default for the error log dir
>
> This has been a weird issue to wrap my head around, it's really using lots of low-level systems (writing tracebacks to files) so I feel like I've done some weird things in the tests. Feedback and critque welcome + appreciated.
>
>
> Diffs
> -----
>
> src/main/python/apache/aurora/client/cli/__init__.py da9d5b6ba4d22ba1f444341b97bbcfaf7889a4a8
> src/test/python/apache/aurora/client/cli/BUILD d33e86643a59879c115876c98bd1dc19aa7ae61c
> src/test/python/apache/aurora/client/cli/test_aurora_command_line.py PRE-CREATION
>
> Diff: https://reviews.apache.org/r/26802/diff/
>
>
> Testing
> -------
>
> [tw-172-25-132-201 aurora (yasumoto/error_log_dir_default)]$ ./pants src/test/python/apache/aurora/client/cli:aurora_command_line
> Build operating on top level addresses: set([BuildFileAddress(/Users/jsmith/workspace/aurora/src/test/python/apache/aurora/client/cli/BUILD, aurora_command_line)])
> ==================================================================== test session starts =====================================================================
> platform darwin -- Python 2.7.5 -- py-1.4.25 -- pytest-2.6.3
> plugins: cov, timeout
> collected 1 items
>
> src/test/python/apache/aurora/client/cli/test_aurora_command_line.py .
>
> ================================================================== 1 passed in 0.63 seconds ==================================================================
> src.test.python.apache.aurora.client.cli.aurora_command_line ..... SUCCESS
>
>
> Thanks,
>
> Joe Smith
>
>
Re: Review Request 26802: Set a default for the error log dir
Posted by Maxim Khutornenko <ma...@apache.org>.
> On Oct. 16, 2014, 5 p.m., Bill Farner wrote:
> > src/test/python/apache/aurora/client/cli/test_aurora_command_line.py, line 34
> > <https://reviews.apache.org/r/26802/diff/1/?file=722892#file722892line34>
> >
> > I'm not sure what the right answer is here, and we may be too far down the monkeypatch road to change tacks in this effort, but overriding builtins feels like a special kind of abstraction violation.
> >
> > Do any other reviewers know the lay of the land well enough to suggest a better way? Ideally we would be injecting this functionallity rather than patching it, but i don't want to turn a small fix into a giant yak shave.
Seems like the appropriate use of patching given the current implementation. Refactoring here would only hurt readability and would rather feel unnatural.
- Maxim
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26802/#review56929
-----------------------------------------------------------
On Oct. 16, 2014, 6:02 a.m., Joe Smith wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26802/
> -----------------------------------------------------------
>
> (Updated Oct. 16, 2014, 6:02 a.m.)
>
>
> Review request for Aurora, Maxim Khutornenko, Bill Farner, and Zameer Manji.
>
>
> Bugs: AURORA-827
> https://issues.apache.org/jira/browse/AURORA-827
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Set a default for the error log dir
>
> This has been a weird issue to wrap my head around, it's really using lots of low-level systems (writing tracebacks to files) so I feel like I've done some weird things in the tests. Feedback and critque welcome + appreciated.
>
>
> Diffs
> -----
>
> src/main/python/apache/aurora/client/cli/__init__.py da9d5b6ba4d22ba1f444341b97bbcfaf7889a4a8
> src/test/python/apache/aurora/client/cli/BUILD d33e86643a59879c115876c98bd1dc19aa7ae61c
> src/test/python/apache/aurora/client/cli/test_aurora_command_line.py PRE-CREATION
>
> Diff: https://reviews.apache.org/r/26802/diff/
>
>
> Testing
> -------
>
> [tw-172-25-132-201 aurora (yasumoto/error_log_dir_default)]$ ./pants src/test/python/apache/aurora/client/cli:aurora_command_line
> Build operating on top level addresses: set([BuildFileAddress(/Users/jsmith/workspace/aurora/src/test/python/apache/aurora/client/cli/BUILD, aurora_command_line)])
> ==================================================================== test session starts =====================================================================
> platform darwin -- Python 2.7.5 -- py-1.4.25 -- pytest-2.6.3
> plugins: cov, timeout
> collected 1 items
>
> src/test/python/apache/aurora/client/cli/test_aurora_command_line.py .
>
> ================================================================== 1 passed in 0.63 seconds ==================================================================
> src.test.python.apache.aurora.client.cli.aurora_command_line ..... SUCCESS
>
>
> Thanks,
>
> Joe Smith
>
>
Re: Review Request 26802: Set a default for the error log dir
Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26802/#review56929
-----------------------------------------------------------
src/main/python/apache/aurora/client/cli/__init__.py
<https://reviews.apache.org/r/26802/#comment97368>
Can you try to remove noqa, and if it is suppressing an error, leave a comment indicating why the pyflakes errors are suppressed?
src/main/python/apache/aurora/client/cli/__init__.py
<https://reviews.apache.org/r/26802/#comment97369>
Ditto
also, more concise:
return Context.ERROR_LOG_DIR or '.'
src/test/python/apache/aurora/client/cli/BUILD
<https://reviews.apache.org/r/26802/#comment97372>
remove extra newline
src/test/python/apache/aurora/client/cli/test_aurora_command_line.py
<https://reviews.apache.org/r/26802/#comment97375>
I'm not sure what the right answer is here, and we may be too far down the monkeypatch road to change tacks in this effort, but overriding builtins feels like a special kind of abstraction violation.
Do any other reviewers know the lay of the land well enough to suggest a better way? Ideally we would be injecting this functionallity rather than patching it, but i don't want to turn a small fix into a giant yak shave.
- Bill Farner
On Oct. 16, 2014, 6:02 a.m., Joe Smith wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26802/
> -----------------------------------------------------------
>
> (Updated Oct. 16, 2014, 6:02 a.m.)
>
>
> Review request for Aurora, Maxim Khutornenko, Bill Farner, and Zameer Manji.
>
>
> Bugs: AURORA-827
> https://issues.apache.org/jira/browse/AURORA-827
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Set a default for the error log dir
>
> This has been a weird issue to wrap my head around, it's really using lots of low-level systems (writing tracebacks to files) so I feel like I've done some weird things in the tests. Feedback and critque welcome + appreciated.
>
>
> Diffs
> -----
>
> src/main/python/apache/aurora/client/cli/__init__.py da9d5b6ba4d22ba1f444341b97bbcfaf7889a4a8
> src/test/python/apache/aurora/client/cli/BUILD d33e86643a59879c115876c98bd1dc19aa7ae61c
> src/test/python/apache/aurora/client/cli/test_aurora_command_line.py PRE-CREATION
>
> Diff: https://reviews.apache.org/r/26802/diff/
>
>
> Testing
> -------
>
> [tw-172-25-132-201 aurora (yasumoto/error_log_dir_default)]$ ./pants src/test/python/apache/aurora/client/cli:aurora_command_line
> Build operating on top level addresses: set([BuildFileAddress(/Users/jsmith/workspace/aurora/src/test/python/apache/aurora/client/cli/BUILD, aurora_command_line)])
> ==================================================================== test session starts =====================================================================
> platform darwin -- Python 2.7.5 -- py-1.4.25 -- pytest-2.6.3
> plugins: cov, timeout
> collected 1 items
>
> src/test/python/apache/aurora/client/cli/test_aurora_command_line.py .
>
> ================================================================== 1 passed in 0.63 seconds ==================================================================
> src.test.python.apache.aurora.client.cli.aurora_command_line ..... SUCCESS
>
>
> Thanks,
>
> Joe Smith
>
>
Re: Review Request 26802: Set a default for the error log dir
Posted by Joe Smith <ya...@gmail.com>.
> On Oct. 16, 2014, 10:24 a.m., Maxim Khutornenko wrote:
> > src/test/python/apache/aurora/client/cli/test_aurora_command_line.py, line 31
> > <https://reviews.apache.org/r/26802/diff/1/?file=722892#file722892line31>
> >
> > Expand 'm' to something more meaningful?
Done
- Joe
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26802/#review56942
-----------------------------------------------------------
On Oct. 15, 2014, 11:02 p.m., Joe Smith wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26802/
> -----------------------------------------------------------
>
> (Updated Oct. 15, 2014, 11:02 p.m.)
>
>
> Review request for Aurora, Maxim Khutornenko, Bill Farner, and Zameer Manji.
>
>
> Bugs: AURORA-827
> https://issues.apache.org/jira/browse/AURORA-827
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Set a default for the error log dir
>
> This has been a weird issue to wrap my head around, it's really using lots of low-level systems (writing tracebacks to files) so I feel like I've done some weird things in the tests. Feedback and critque welcome + appreciated.
>
>
> Diffs
> -----
>
> src/main/python/apache/aurora/client/cli/__init__.py da9d5b6ba4d22ba1f444341b97bbcfaf7889a4a8
> src/test/python/apache/aurora/client/cli/BUILD d33e86643a59879c115876c98bd1dc19aa7ae61c
> src/test/python/apache/aurora/client/cli/test_aurora_command_line.py PRE-CREATION
>
> Diff: https://reviews.apache.org/r/26802/diff/
>
>
> Testing
> -------
>
> [tw-172-25-132-201 aurora (yasumoto/error_log_dir_default)]$ ./pants src/test/python/apache/aurora/client/cli:aurora_command_line
> Build operating on top level addresses: set([BuildFileAddress(/Users/jsmith/workspace/aurora/src/test/python/apache/aurora/client/cli/BUILD, aurora_command_line)])
> ==================================================================== test session starts =====================================================================
> platform darwin -- Python 2.7.5 -- py-1.4.25 -- pytest-2.6.3
> plugins: cov, timeout
> collected 1 items
>
> src/test/python/apache/aurora/client/cli/test_aurora_command_line.py .
>
> ================================================================== 1 passed in 0.63 seconds ==================================================================
> src.test.python.apache.aurora.client.cli.aurora_command_line ..... SUCCESS
>
>
> Thanks,
>
> Joe Smith
>
>
Re: Review Request 26802: Set a default for the error log dir
Posted by Maxim Khutornenko <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26802/#review56942
-----------------------------------------------------------
Ship it!
src/test/python/apache/aurora/client/cli/test_aurora_command_line.py
<https://reviews.apache.org/r/26802/#comment97383>
Expand 'm' to something more meaningful?
- Maxim Khutornenko
On Oct. 16, 2014, 6:02 a.m., Joe Smith wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26802/
> -----------------------------------------------------------
>
> (Updated Oct. 16, 2014, 6:02 a.m.)
>
>
> Review request for Aurora, Maxim Khutornenko, Bill Farner, and Zameer Manji.
>
>
> Bugs: AURORA-827
> https://issues.apache.org/jira/browse/AURORA-827
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Set a default for the error log dir
>
> This has been a weird issue to wrap my head around, it's really using lots of low-level systems (writing tracebacks to files) so I feel like I've done some weird things in the tests. Feedback and critque welcome + appreciated.
>
>
> Diffs
> -----
>
> src/main/python/apache/aurora/client/cli/__init__.py da9d5b6ba4d22ba1f444341b97bbcfaf7889a4a8
> src/test/python/apache/aurora/client/cli/BUILD d33e86643a59879c115876c98bd1dc19aa7ae61c
> src/test/python/apache/aurora/client/cli/test_aurora_command_line.py PRE-CREATION
>
> Diff: https://reviews.apache.org/r/26802/diff/
>
>
> Testing
> -------
>
> [tw-172-25-132-201 aurora (yasumoto/error_log_dir_default)]$ ./pants src/test/python/apache/aurora/client/cli:aurora_command_line
> Build operating on top level addresses: set([BuildFileAddress(/Users/jsmith/workspace/aurora/src/test/python/apache/aurora/client/cli/BUILD, aurora_command_line)])
> ==================================================================== test session starts =====================================================================
> platform darwin -- Python 2.7.5 -- py-1.4.25 -- pytest-2.6.3
> plugins: cov, timeout
> collected 1 items
>
> src/test/python/apache/aurora/client/cli/test_aurora_command_line.py .
>
> ================================================================== 1 passed in 0.63 seconds ==================================================================
> src.test.python.apache.aurora.client.cli.aurora_command_line ..... SUCCESS
>
>
> Thanks,
>
> Joe Smith
>
>
Re: Review Request 26802: Set a default for the error log dir
Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26802/#review56994
-----------------------------------------------------------
Tests failed:
src/test/python/apache/aurora/client/cli/test_create.py::TestClientCreateCommand::test_create_job_fail_and_write_log Fatal error running command; traceback can be found in ./aurora-23.error-log
FAILED
============================================================================================================================ FAILURES ============================================================================================================================
___________________________________________________________________________________________________ TestClientCreateCommand.test_create_job_fail_and_write_log ___________________________________________________________________________________________________
self = <test_create.TestClientCreateCommand testMethod=test_create_job_fail_and_write_log>
def test_create_job_fail_and_write_log(self):
"""Check that when an unknown error occurs during command execution,
the command-line framework catches it, and writes out an error log file
containing the details of the error, including the command-line arguments
passed to aurora to execute the command, and the stack trace of the error.
"""
mock_context = FakeAuroraCommandContext()
with contextlib.nested(
patch('time.time', return_value=23),
patch('apache.aurora.client.cli.jobs.Job.create_context', return_value=mock_context)):
api = mock_context.get_api('west')
api.create_job.side_effect = UnknownException()
with temporary_file() as fp:
fp.write(self.get_valid_config())
fp.flush()
cmd = AuroraCommandLine()
result = cmd.execute(['job', 'create', '--wait-until=RUNNING',
'--error-log-dir=./logged-errors', 'west/bozo/test/hello',
fp.name])
assert result == EXIT_UNKNOWN_ERROR
> with open("./logged-errors/aurora-23.error-log", "r") as logfile:
error_log = logfile.read()
E IOError: [Errno 2] No such file or directory: './logged-errors/aurora-23.error-log'
Within `~/.aurora/errors/aurora-23.error-log` (note that it is not `./aurora-23.error-log` as the client output purports:
ERROR LOG: command arguments = ['job', 'create', '--wait-until=RUNNING', 'west/bozo/test/hello', '--error-log-dir=./error-logs', '/tmp/tmp0p7ioZ']
Traceback (most recent call last):
File "/tmp/tmpA9GsN3/apache/aurora/client/cli/__init__.py", line 446, in execute
return self._execute(args)
File "/tmp/tmpA9GsN3/apache/aurora/client/cli/__init__.py", line 400, in _execute
noun, context = self._parse_args(args)
File "/tmp/tmpA9GsN3/apache/aurora/client/cli/__init__.py", line 357, in _parse_args
context = noun.create_context()
File "/tmp/tmpA9GsN3/.deps/mock-1.0.1-py2-none-any.whl/mock.py", line 955, in __call__
return _mock_self._mock_call(*args, **kwargs)
File "/tmp/tmpA9GsN3/.deps/mock-1.0.1-py2-none-any.whl/mock.py", line 1010, in _mock_call
raise effect
Exception: Argh
- Bill Farner
On Oct. 16, 2014, 8:22 p.m., Joe Smith wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26802/
> -----------------------------------------------------------
>
> (Updated Oct. 16, 2014, 8:22 p.m.)
>
>
> Review request for Aurora, Maxim Khutornenko and Bill Farner.
>
>
> Bugs: AURORA-827
> https://issues.apache.org/jira/browse/AURORA-827
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Set a default for the error log dir
>
> This has been a weird issue to wrap my head around, it's really using lots of low-level systems (writing tracebacks to files) so I feel like I've done some weird things in the tests. Feedback and critque welcome + appreciated.
>
>
> Diffs
> -----
>
> src/main/python/apache/aurora/client/cli/__init__.py da9d5b6ba4d22ba1f444341b97bbcfaf7889a4a8
> src/test/python/apache/aurora/client/cli/BUILD d33e86643a59879c115876c98bd1dc19aa7ae61c
> src/test/python/apache/aurora/client/cli/test_aurora_command_line.py PRE-CREATION
>
> Diff: https://reviews.apache.org/r/26802/diff/
>
>
> Testing
> -------
>
> [tw-172-25-132-201 aurora (yasumoto/error_log_dir_default)]$ ./pants src/test/python/apache/aurora/client/cli:aurora_command_line
> Build operating on top level addresses: set([BuildFileAddress(/Users/jsmith/workspace/aurora/src/test/python/apache/aurora/client/cli/BUILD, aurora_command_line)])
> ==================================================================== test session starts =====================================================================
> platform darwin -- Python 2.7.5 -- py-1.4.25 -- pytest-2.6.3
> plugins: cov, timeout
> collected 1 items
>
> src/test/python/apache/aurora/client/cli/test_aurora_command_line.py .
>
> ================================================================== 1 passed in 0.63 seconds ==================================================================
> src.test.python.apache.aurora.client.cli.aurora_command_line ..... SUCCESS
>
>
> Thanks,
>
> Joe Smith
>
>
Re: Review Request 26802: Set a default for the error log dir
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26802/#review58217
-----------------------------------------------------------
This patch does not apply cleanly on master (53f4e73), do you need to rebase?
- Aurora ReviewBot
On Oct. 16, 2014, 8:22 p.m., Joe Smith wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26802/
> -----------------------------------------------------------
>
> (Updated Oct. 16, 2014, 8:22 p.m.)
>
>
> Review request for Aurora, Maxim Khutornenko and Bill Farner.
>
>
> Bugs: AURORA-827
> https://issues.apache.org/jira/browse/AURORA-827
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Set a default for the error log dir
>
> This has been a weird issue to wrap my head around, it's really using lots of low-level systems (writing tracebacks to files) so I feel like I've done some weird things in the tests. Feedback and critque welcome + appreciated.
>
>
> Diffs
> -----
>
> src/main/python/apache/aurora/client/cli/__init__.py da9d5b6ba4d22ba1f444341b97bbcfaf7889a4a8
> src/test/python/apache/aurora/client/cli/BUILD d33e86643a59879c115876c98bd1dc19aa7ae61c
> src/test/python/apache/aurora/client/cli/test_aurora_command_line.py PRE-CREATION
>
> Diff: https://reviews.apache.org/r/26802/diff/
>
>
> Testing
> -------
>
> [tw-172-25-132-201 aurora (yasumoto/error_log_dir_default)]$ ./pants src/test/python/apache/aurora/client/cli:aurora_command_line
> Build operating on top level addresses: set([BuildFileAddress(/Users/jsmith/workspace/aurora/src/test/python/apache/aurora/client/cli/BUILD, aurora_command_line)])
> ==================================================================== test session starts =====================================================================
> platform darwin -- Python 2.7.5 -- py-1.4.25 -- pytest-2.6.3
> plugins: cov, timeout
> collected 1 items
>
> src/test/python/apache/aurora/client/cli/test_aurora_command_line.py .
>
> ================================================================== 1 passed in 0.63 seconds ==================================================================
> src.test.python.apache.aurora.client.cli.aurora_command_line ..... SUCCESS
>
>
> Thanks,
>
> Joe Smith
>
>
Re: Review Request 26802: Set a default for the error log dir
Posted by Joe Smith <ya...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26802/
-----------------------------------------------------------
(Updated Oct. 16, 2014, 1:22 p.m.)
Review request for Aurora, Maxim Khutornenko and Bill Farner.
Changes
-------
Removing zmanji per request (thanks for being upfront!)
Bugs: AURORA-827
https://issues.apache.org/jira/browse/AURORA-827
Repository: aurora
Description
-------
Set a default for the error log dir
This has been a weird issue to wrap my head around, it's really using lots of low-level systems (writing tracebacks to files) so I feel like I've done some weird things in the tests. Feedback and critque welcome + appreciated.
Diffs
-----
src/main/python/apache/aurora/client/cli/__init__.py da9d5b6ba4d22ba1f444341b97bbcfaf7889a4a8
src/test/python/apache/aurora/client/cli/BUILD d33e86643a59879c115876c98bd1dc19aa7ae61c
src/test/python/apache/aurora/client/cli/test_aurora_command_line.py PRE-CREATION
Diff: https://reviews.apache.org/r/26802/diff/
Testing
-------
[tw-172-25-132-201 aurora (yasumoto/error_log_dir_default)]$ ./pants src/test/python/apache/aurora/client/cli:aurora_command_line
Build operating on top level addresses: set([BuildFileAddress(/Users/jsmith/workspace/aurora/src/test/python/apache/aurora/client/cli/BUILD, aurora_command_line)])
==================================================================== test session starts =====================================================================
platform darwin -- Python 2.7.5 -- py-1.4.25 -- pytest-2.6.3
plugins: cov, timeout
collected 1 items
src/test/python/apache/aurora/client/cli/test_aurora_command_line.py .
================================================================== 1 passed in 0.63 seconds ==================================================================
src.test.python.apache.aurora.client.cli.aurora_command_line ..... SUCCESS
Thanks,
Joe Smith
Re: Review Request 26802: Set a default for the error log dir
Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26802/#review56981
-----------------------------------------------------------
Ship it!
Ship It!
- Bill Farner
On Oct. 16, 2014, 5:51 p.m., Joe Smith wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26802/
> -----------------------------------------------------------
>
> (Updated Oct. 16, 2014, 5:51 p.m.)
>
>
> Review request for Aurora, Maxim Khutornenko, Bill Farner, and Zameer Manji.
>
>
> Bugs: AURORA-827
> https://issues.apache.org/jira/browse/AURORA-827
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Set a default for the error log dir
>
> This has been a weird issue to wrap my head around, it's really using lots of low-level systems (writing tracebacks to files) so I feel like I've done some weird things in the tests. Feedback and critque welcome + appreciated.
>
>
> Diffs
> -----
>
> src/main/python/apache/aurora/client/cli/__init__.py da9d5b6ba4d22ba1f444341b97bbcfaf7889a4a8
> src/test/python/apache/aurora/client/cli/BUILD d33e86643a59879c115876c98bd1dc19aa7ae61c
> src/test/python/apache/aurora/client/cli/test_aurora_command_line.py PRE-CREATION
>
> Diff: https://reviews.apache.org/r/26802/diff/
>
>
> Testing
> -------
>
> [tw-172-25-132-201 aurora (yasumoto/error_log_dir_default)]$ ./pants src/test/python/apache/aurora/client/cli:aurora_command_line
> Build operating on top level addresses: set([BuildFileAddress(/Users/jsmith/workspace/aurora/src/test/python/apache/aurora/client/cli/BUILD, aurora_command_line)])
> ==================================================================== test session starts =====================================================================
> platform darwin -- Python 2.7.5 -- py-1.4.25 -- pytest-2.6.3
> plugins: cov, timeout
> collected 1 items
>
> src/test/python/apache/aurora/client/cli/test_aurora_command_line.py .
>
> ================================================================== 1 passed in 0.63 seconds ==================================================================
> src.test.python.apache.aurora.client.cli.aurora_command_line ..... SUCCESS
>
>
> Thanks,
>
> Joe Smith
>
>
Re: Review Request 26802: Set a default for the error log dir
Posted by Zameer Manji <zm...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26802/#review56983
-----------------------------------------------------------
I have no time to review this. Please remove me from review.
- Zameer Manji
On Oct. 16, 2014, 10:51 a.m., Joe Smith wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26802/
> -----------------------------------------------------------
>
> (Updated Oct. 16, 2014, 10:51 a.m.)
>
>
> Review request for Aurora, Maxim Khutornenko, Bill Farner, and Zameer Manji.
>
>
> Bugs: AURORA-827
> https://issues.apache.org/jira/browse/AURORA-827
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Set a default for the error log dir
>
> This has been a weird issue to wrap my head around, it's really using lots of low-level systems (writing tracebacks to files) so I feel like I've done some weird things in the tests. Feedback and critque welcome + appreciated.
>
>
> Diffs
> -----
>
> src/main/python/apache/aurora/client/cli/__init__.py da9d5b6ba4d22ba1f444341b97bbcfaf7889a4a8
> src/test/python/apache/aurora/client/cli/BUILD d33e86643a59879c115876c98bd1dc19aa7ae61c
> src/test/python/apache/aurora/client/cli/test_aurora_command_line.py PRE-CREATION
>
> Diff: https://reviews.apache.org/r/26802/diff/
>
>
> Testing
> -------
>
> [tw-172-25-132-201 aurora (yasumoto/error_log_dir_default)]$ ./pants src/test/python/apache/aurora/client/cli:aurora_command_line
> Build operating on top level addresses: set([BuildFileAddress(/Users/jsmith/workspace/aurora/src/test/python/apache/aurora/client/cli/BUILD, aurora_command_line)])
> ==================================================================== test session starts =====================================================================
> platform darwin -- Python 2.7.5 -- py-1.4.25 -- pytest-2.6.3
> plugins: cov, timeout
> collected 1 items
>
> src/test/python/apache/aurora/client/cli/test_aurora_command_line.py .
>
> ================================================================== 1 passed in 0.63 seconds ==================================================================
> src.test.python.apache.aurora.client.cli.aurora_command_line ..... SUCCESS
>
>
> Thanks,
>
> Joe Smith
>
>
Re: Review Request 26802: Set a default for the error log dir
Posted by Joe Smith <ya...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26802/
-----------------------------------------------------------
(Updated Oct. 16, 2014, 10:51 a.m.)
Review request for Aurora, Maxim Khutornenko, Bill Farner, and Zameer Manji.
Changes
-------
Review feedback (thanks!)
Bugs: AURORA-827
https://issues.apache.org/jira/browse/AURORA-827
Repository: aurora
Description
-------
Set a default for the error log dir
This has been a weird issue to wrap my head around, it's really using lots of low-level systems (writing tracebacks to files) so I feel like I've done some weird things in the tests. Feedback and critque welcome + appreciated.
Diffs (updated)
-----
src/main/python/apache/aurora/client/cli/__init__.py da9d5b6ba4d22ba1f444341b97bbcfaf7889a4a8
src/test/python/apache/aurora/client/cli/BUILD d33e86643a59879c115876c98bd1dc19aa7ae61c
src/test/python/apache/aurora/client/cli/test_aurora_command_line.py PRE-CREATION
Diff: https://reviews.apache.org/r/26802/diff/
Testing
-------
[tw-172-25-132-201 aurora (yasumoto/error_log_dir_default)]$ ./pants src/test/python/apache/aurora/client/cli:aurora_command_line
Build operating on top level addresses: set([BuildFileAddress(/Users/jsmith/workspace/aurora/src/test/python/apache/aurora/client/cli/BUILD, aurora_command_line)])
==================================================================== test session starts =====================================================================
platform darwin -- Python 2.7.5 -- py-1.4.25 -- pytest-2.6.3
plugins: cov, timeout
collected 1 items
src/test/python/apache/aurora/client/cli/test_aurora_command_line.py .
================================================================== 1 passed in 0.63 seconds ==================================================================
src.test.python.apache.aurora.client.cli.aurora_command_line ..... SUCCESS
Thanks,
Joe Smith