You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Gastón Kleiman <ga...@mesosphere.io> on 2017/06/07 05:17:21 UTC
Review Request 59874: Made `CheckerProcess` use a consistent
description for log messages.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59874/
-----------------------------------------------------------
Review request for mesos, Alexander Rukletsov and Vinod Kone.
Bugs: MESOS-7092
https://issues.apache.org/jira/browse/MESOS-7092
Repository: mesos
Description
-------
Made `CheckerProcess` use a consistent description for log messages.
Diffs
-----
src/checks/checker.cpp dcc3164f3b623de73bacf024ede95b40c48f7192
src/checks/checker_process.hpp PRE-CREATION
src/checks/checker_process.cpp PRE-CREATION
src/checks/health_checker.cpp 9d8c8471caa05af3908d34315dbfed08a343c0f8
Diff: https://reviews.apache.org/r/59874/diff/1/
Testing
-------
Tests still pass on GNU/Linux.
Thanks,
Gastón Kleiman
Re: Review Request 59874: Made `CheckerProcess` use a consistent
description for log messages.
Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59874/#review177149
-----------------------------------------------------------
Bad patch!
Reviews applied: [59874, 59873, 59872, 59871]
Failed command: python support/apply-reviews.py -n -r 59871
Error:
Traceback (most recent call last):
File "support/apply-reviews.py", line 381, in <module>
reviewboard()
File "support/apply-reviews.py", line 360, in reviewboard
apply_review()
File "support/apply-reviews.py", line 126, in apply_review
commit_patch()
File "support/apply-reviews.py", line 225, in commit_patch
shell(cmd, options['dry_run'])
File "support/apply-reviews.py", line 111, in shell
error_code = subprocess.call(command, stderr=subprocess.STDOUT, shell=True)
File "C:\Python27\lib\subprocess.py", line 168, in call
return Popen(*popenargs, **kwargs).wait()
File "C:\Python27\lib\subprocess.py", line 390, in __init__
errread, errwrite)
File "C:\Python27\lib\subprocess.py", line 610, in _execute_child
args = '{} /c "{}"'.format (comspec, args)
UnicodeEncodeError: 'ascii' codec can't encode character u'\xf3' in position 25: ordinal not in range(128)
Full log: http://mesos-winbot.westus.cloudapp.azure.com/logs/66/console
- Mesos Reviewbot Windows
On June 7, 2017, 5:17 a.m., Gastón Kleiman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59874/
> -----------------------------------------------------------
>
> (Updated June 7, 2017, 5:17 a.m.)
>
>
> Review request for mesos, Alexander Rukletsov and Vinod Kone.
>
>
> Bugs: MESOS-7092
> https://issues.apache.org/jira/browse/MESOS-7092
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Made `CheckerProcess` use a consistent description for log messages.
>
>
> Diffs
> -----
>
> src/checks/checker.cpp dcc3164f3b623de73bacf024ede95b40c48f7192
> src/checks/checker_process.hpp PRE-CREATION
> src/checks/checker_process.cpp PRE-CREATION
> src/checks/health_checker.cpp 9d8c8471caa05af3908d34315dbfed08a343c0f8
>
>
> Diff: https://reviews.apache.org/r/59874/diff/1/
>
>
> Testing
> -------
>
> Tests still pass on GNU/Linux.
>
>
> Thanks,
>
> Gastón Kleiman
>
>
Re: Review Request 59874: Made `CheckerProcess` use a consistent
description for log messages.
Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59874/#review177300
-----------------------------------------------------------
src/checks/checker.cpp
Lines 176 (patched)
<https://reviews.apache.org/r/59874/#comment250826>
I suggest to cache `name` also in `Checker` and use it there in logs. Ditto for `HealthChecker`.
src/checks/checker_process.cpp
Lines 921-923 (original), 936-938 (patched)
<https://reviews.apache.org/r/59874/#comment250821>
4 spaces indent please.
- Alexander Rukletsov
On June 8, 2017, 12:26 a.m., Gastón Kleiman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59874/
> -----------------------------------------------------------
>
> (Updated June 8, 2017, 12:26 a.m.)
>
>
> Review request for mesos, Alexander Rukletsov and Vinod Kone.
>
>
> Bugs: MESOS-7092
> https://issues.apache.org/jira/browse/MESOS-7092
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Made `CheckerProcess` use a consistent description for log messages.
>
>
> Diffs
> -----
>
> src/checks/checker.cpp dcc3164f3b623de73bacf024ede95b40c48f7192
> src/checks/checker_process.hpp PRE-CREATION
> src/checks/checker_process.cpp PRE-CREATION
> src/checks/health_checker.cpp 9d8c8471caa05af3908d34315dbfed08a343c0f8
>
>
> Diff: https://reviews.apache.org/r/59874/diff/2/
>
>
> Testing
> -------
>
> Tests still pass on GNU/Linux.
>
>
> Thanks,
>
> Gastón Kleiman
>
>
Re: Review Request 59874: Made `CheckerProcess` use a consistent
description for log messages.
Posted by Alexander Rukletsov <ru...@gmail.com>.
> On June 9, 2017, 3:48 a.m., Mesos Reviewbot Windows wrote:
> > Bad patch!
> >
> > Reviews applied: [59874, 59902, 59873, 59872, 59871]
> >
> > Failed command: python support/apply-reviews.py -n -r 59871
> >
> > Error:
> > Traceback (most recent call last):
> > File "support/apply-reviews.py", line 381, in <module>
> > reviewboard()
> > File "support/apply-reviews.py", line 360, in reviewboard
> > apply_review()
> > File "support/apply-reviews.py", line 126, in apply_review
> > commit_patch()
> > File "support/apply-reviews.py", line 225, in commit_patch
> > shell(cmd, options['dry_run'])
> > File "support/apply-reviews.py", line 111, in shell
> > error_code = subprocess.call(command, stderr=subprocess.STDOUT, shell=True)
> > File "C:\Python27\lib\subprocess.py", line 168, in call
> > return Popen(*popenargs, **kwargs).wait()
> > File "C:\Python27\lib\subprocess.py", line 390, in __init__
> > errread, errwrite)
> > File "C:\Python27\lib\subprocess.py", line 610, in _execute_child
> > args = '{} /c "{}"'.format (comspec, args)
> > UnicodeEncodeError: 'ascii' codec can't encode character u'\xf3' in position 25: ordinal not in range(128)
> >
> > Full log: http://mesos-winbot.westus.cloudapp.azure.com/logs/80/console
Not sure what this means. I'll ask around but ignore it for now.
- Alexander
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59874/#review177435
-----------------------------------------------------------
On June 9, 2017, 3:43 a.m., Gastón Kleiman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59874/
> -----------------------------------------------------------
>
> (Updated June 9, 2017, 3:43 a.m.)
>
>
> Review request for mesos, Alexander Rukletsov and Vinod Kone.
>
>
> Bugs: MESOS-7092
> https://issues.apache.org/jira/browse/MESOS-7092
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Made `CheckerProcess` use a consistent description for log messages.
>
>
> Diffs
> -----
>
> src/checks/checker.cpp dcc3164f3b623de73bacf024ede95b40c48f7192
> src/checks/checker_process.hpp PRE-CREATION
> src/checks/checker_process.cpp PRE-CREATION
> src/checks/health_checker.cpp 9d8c8471caa05af3908d34315dbfed08a343c0f8
>
>
> Diff: https://reviews.apache.org/r/59874/diff/3/
>
>
> Testing
> -------
>
> Tests still pass on GNU/Linux.
>
>
> Thanks,
>
> Gastón Kleiman
>
>
Re: Review Request 59874: Made `CheckerProcess` use a consistent
description for log messages.
Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59874/#review177435
-----------------------------------------------------------
Bad patch!
Reviews applied: [59874, 59902, 59873, 59872, 59871]
Failed command: python support/apply-reviews.py -n -r 59871
Error:
Traceback (most recent call last):
File "support/apply-reviews.py", line 381, in <module>
reviewboard()
File "support/apply-reviews.py", line 360, in reviewboard
apply_review()
File "support/apply-reviews.py", line 126, in apply_review
commit_patch()
File "support/apply-reviews.py", line 225, in commit_patch
shell(cmd, options['dry_run'])
File "support/apply-reviews.py", line 111, in shell
error_code = subprocess.call(command, stderr=subprocess.STDOUT, shell=True)
File "C:\Python27\lib\subprocess.py", line 168, in call
return Popen(*popenargs, **kwargs).wait()
File "C:\Python27\lib\subprocess.py", line 390, in __init__
errread, errwrite)
File "C:\Python27\lib\subprocess.py", line 610, in _execute_child
args = '{} /c "{}"'.format (comspec, args)
UnicodeEncodeError: 'ascii' codec can't encode character u'\xf3' in position 25: ordinal not in range(128)
Full log: http://mesos-winbot.westus.cloudapp.azure.com/logs/80/console
- Mesos Reviewbot Windows
On June 9, 2017, 6:43 a.m., Gastón Kleiman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59874/
> -----------------------------------------------------------
>
> (Updated June 9, 2017, 6:43 a.m.)
>
>
> Review request for mesos, Alexander Rukletsov and Vinod Kone.
>
>
> Bugs: MESOS-7092
> https://issues.apache.org/jira/browse/MESOS-7092
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Made `CheckerProcess` use a consistent description for log messages.
>
>
> Diffs
> -----
>
> src/checks/checker.cpp dcc3164f3b623de73bacf024ede95b40c48f7192
> src/checks/checker_process.hpp PRE-CREATION
> src/checks/checker_process.cpp PRE-CREATION
> src/checks/health_checker.cpp 9d8c8471caa05af3908d34315dbfed08a343c0f8
>
>
> Diff: https://reviews.apache.org/r/59874/diff/3/
>
>
> Testing
> -------
>
> Tests still pass on GNU/Linux.
>
>
> Thanks,
>
> Gastón Kleiman
>
>
Re: Review Request 59874: Made `CheckerProcess` use a consistent
description for log messages.
Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59874/#review177478
-----------------------------------------------------------
Ship it!
Ship It!
- Alexander Rukletsov
On June 9, 2017, 3:43 a.m., Gastón Kleiman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59874/
> -----------------------------------------------------------
>
> (Updated June 9, 2017, 3:43 a.m.)
>
>
> Review request for mesos, Alexander Rukletsov and Vinod Kone.
>
>
> Bugs: MESOS-7092
> https://issues.apache.org/jira/browse/MESOS-7092
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Made `CheckerProcess` use a consistent description for log messages.
>
>
> Diffs
> -----
>
> src/checks/checker.cpp dcc3164f3b623de73bacf024ede95b40c48f7192
> src/checks/checker_process.hpp PRE-CREATION
> src/checks/checker_process.cpp PRE-CREATION
> src/checks/health_checker.cpp 9d8c8471caa05af3908d34315dbfed08a343c0f8
>
>
> Diff: https://reviews.apache.org/r/59874/diff/3/
>
>
> Testing
> -------
>
> Tests still pass on GNU/Linux.
>
>
> Thanks,
>
> Gastón Kleiman
>
>
Re: Review Request 59874: Made `CheckerProcess` use a consistent
description for log messages.
Posted by Gastón Kleiman <ga...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59874/
-----------------------------------------------------------
(Updated June 9, 2017, 3:43 a.m.)
Review request for mesos, Alexander Rukletsov and Vinod Kone.
Changes
-------
Addressed feedback.
Bugs: MESOS-7092
https://issues.apache.org/jira/browse/MESOS-7092
Repository: mesos
Description
-------
Made `CheckerProcess` use a consistent description for log messages.
Diffs (updated)
-----
src/checks/checker.cpp dcc3164f3b623de73bacf024ede95b40c48f7192
src/checks/checker_process.hpp PRE-CREATION
src/checks/checker_process.cpp PRE-CREATION
src/checks/health_checker.cpp 9d8c8471caa05af3908d34315dbfed08a343c0f8
Diff: https://reviews.apache.org/r/59874/diff/3/
Changes: https://reviews.apache.org/r/59874/diff/2-3/
Testing
-------
Tests still pass on GNU/Linux.
Thanks,
Gastón Kleiman
Re: Review Request 59874: Made `CheckerProcess` use a consistent
description for log messages.
Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59874/#review177269
-----------------------------------------------------------
Bad patch!
Reviews applied: [59874, 59902, 59873, 59872, 59871]
Failed command: python support/apply-reviews.py -n -r 59871
Error:
Traceback (most recent call last):
File "support/apply-reviews.py", line 381, in <module>
reviewboard()
File "support/apply-reviews.py", line 360, in reviewboard
apply_review()
File "support/apply-reviews.py", line 126, in apply_review
commit_patch()
File "support/apply-reviews.py", line 225, in commit_patch
shell(cmd, options['dry_run'])
File "support/apply-reviews.py", line 111, in shell
error_code = subprocess.call(command, stderr=subprocess.STDOUT, shell=True)
File "C:\Python27\lib\subprocess.py", line 168, in call
return Popen(*popenargs, **kwargs).wait()
File "C:\Python27\lib\subprocess.py", line 390, in __init__
errread, errwrite)
File "C:\Python27\lib\subprocess.py", line 610, in _execute_child
args = '{} /c "{}"'.format (comspec, args)
UnicodeEncodeError: 'ascii' codec can't encode character u'\xf3' in position 25: ordinal not in range(128)
Full log: http://mesos-winbot.westus.cloudapp.azure.com/logs/77/console
- Mesos Reviewbot Windows
On June 8, 2017, 12:26 a.m., Gastón Kleiman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59874/
> -----------------------------------------------------------
>
> (Updated June 8, 2017, 12:26 a.m.)
>
>
> Review request for mesos, Alexander Rukletsov and Vinod Kone.
>
>
> Bugs: MESOS-7092
> https://issues.apache.org/jira/browse/MESOS-7092
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Made `CheckerProcess` use a consistent description for log messages.
>
>
> Diffs
> -----
>
> src/checks/checker.cpp dcc3164f3b623de73bacf024ede95b40c48f7192
> src/checks/checker_process.hpp PRE-CREATION
> src/checks/checker_process.cpp PRE-CREATION
> src/checks/health_checker.cpp 9d8c8471caa05af3908d34315dbfed08a343c0f8
>
>
> Diff: https://reviews.apache.org/r/59874/diff/2/
>
>
> Testing
> -------
>
> Tests still pass on GNU/Linux.
>
>
> Thanks,
>
> Gastón Kleiman
>
>
Re: Review Request 59874: Made `CheckerProcess` use a consistent
description for log messages.
Posted by Gastón Kleiman <ga...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59874/
-----------------------------------------------------------
(Updated June 8, 2017, 12:26 a.m.)
Review request for mesos, Alexander Rukletsov and Vinod Kone.
Changes
-------
Addressed feedback.
Bugs: MESOS-7092
https://issues.apache.org/jira/browse/MESOS-7092
Repository: mesos
Description
-------
Made `CheckerProcess` use a consistent description for log messages.
Diffs (updated)
-----
src/checks/checker.cpp dcc3164f3b623de73bacf024ede95b40c48f7192
src/checks/checker_process.hpp PRE-CREATION
src/checks/checker_process.cpp PRE-CREATION
src/checks/health_checker.cpp 9d8c8471caa05af3908d34315dbfed08a343c0f8
Diff: https://reviews.apache.org/r/59874/diff/2/
Changes: https://reviews.apache.org/r/59874/diff/1-2/
Testing
-------
Tests still pass on GNU/Linux.
Thanks,
Gastón Kleiman
Re: Review Request 59874: Made `CheckerProcess` use a consistent
description for log messages.
Posted by Gastón Kleiman <ga...@mesosphere.io>.
> On June 7, 2017, 4:49 p.m., Alexander Rukletsov wrote:
> > src/checks/checker_process.hpp
> > Lines 58 (patched)
> > <https://reviews.apache.org/r/59874/diff/1/?file=1743744#file1743744line58>
> >
> > I'd rather keep `id` (or `name`) in each class and pass it down to `CheckerProcess`.
I don't think we need to keep `name` in each class, but I did update the patch to generate it there and pass it down, instead of generating it in `CheckerProcess`.
> On June 7, 2017, 4:49 p.m., Alexander Rukletsov wrote:
> > src/checks/checker_process.hpp
> > Lines 140 (patched)
> > <https://reviews.apache.org/r/59874/diff/1/?file=1743744#file1743744line140>
> >
> > If you inherited `id` (`name`), you could update it here instead of having a separate `buildName` function for this and an extra `description` field.
I'm saving the name in a different field and not using it as the process ID, because Process IDs are separated by dashes instead of spaces, and that would look pretty bad in the log messages.
- Gastón
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59874/#review177180
-----------------------------------------------------------
On June 8, 2017, 12:26 a.m., Gastón Kleiman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59874/
> -----------------------------------------------------------
>
> (Updated June 8, 2017, 12:26 a.m.)
>
>
> Review request for mesos, Alexander Rukletsov and Vinod Kone.
>
>
> Bugs: MESOS-7092
> https://issues.apache.org/jira/browse/MESOS-7092
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Made `CheckerProcess` use a consistent description for log messages.
>
>
> Diffs
> -----
>
> src/checks/checker.cpp dcc3164f3b623de73bacf024ede95b40c48f7192
> src/checks/checker_process.hpp PRE-CREATION
> src/checks/checker_process.cpp PRE-CREATION
> src/checks/health_checker.cpp 9d8c8471caa05af3908d34315dbfed08a343c0f8
>
>
> Diff: https://reviews.apache.org/r/59874/diff/2/
>
>
> Testing
> -------
>
> Tests still pass on GNU/Linux.
>
>
> Thanks,
>
> Gastón Kleiman
>
>
Re: Review Request 59874: Made `CheckerProcess` use a consistent
description for log messages.
Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59874/#review177180
-----------------------------------------------------------
src/checks/checker_process.hpp
Lines 58 (patched)
<https://reviews.apache.org/r/59874/#comment250726>
I'd rather keep `id` (or `name`) in each class and pass it down to `CheckerProcess`.
src/checks/checker_process.hpp
Lines 140 (patched)
<https://reviews.apache.org/r/59874/#comment250727>
If you inherited `id` (`name`), you could update it here instead of having a separate `buildName` function for this and an extra `description` field.
- Alexander Rukletsov
On June 7, 2017, 5:17 a.m., Gastón Kleiman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59874/
> -----------------------------------------------------------
>
> (Updated June 7, 2017, 5:17 a.m.)
>
>
> Review request for mesos, Alexander Rukletsov and Vinod Kone.
>
>
> Bugs: MESOS-7092
> https://issues.apache.org/jira/browse/MESOS-7092
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Made `CheckerProcess` use a consistent description for log messages.
>
>
> Diffs
> -----
>
> src/checks/checker.cpp dcc3164f3b623de73bacf024ede95b40c48f7192
> src/checks/checker_process.hpp PRE-CREATION
> src/checks/checker_process.cpp PRE-CREATION
> src/checks/health_checker.cpp 9d8c8471caa05af3908d34315dbfed08a343c0f8
>
>
> Diff: https://reviews.apache.org/r/59874/diff/1/
>
>
> Testing
> -------
>
> Tests still pass on GNU/Linux.
>
>
> Thanks,
>
> Gastón Kleiman
>
>