Re: Review Request 62018: Updated logging::initialize to make flags optional.

2017-09-18 Thread Alexander Rukletsov 

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


Ship it!




Ship It!

- Alexander Rukletsov


On Sept. 18, 2017, 1:05 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62018/
> ---
> 
> (Updated Sept. 18, 2017, 1:05 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-7586
> https://issues.apache.org/jira/browse/MESOS-7586
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This will be used to initialize logging in the main functions
> that do not use `mesos::internal::logging::Flags`.
> 
> 
> Diffs
> -
> 
>   src/logging/logging.hpp 0e448f25de6c66d772f5bc481fbaf4aa493fd8af 
>   src/logging/logging.cpp 70d66a5c396f709e8f27ad0d51315ed6d257f73b 
> 
> 
> Diff: https://reviews.apache.org/r/62018/diff/4/
> 
> 
> Testing
> ---
> 
> ```
> $ make check
> ```
> 
> Also tested manually to see if using `LOG()` without initializing GLOG gives 
> the same result as using it after running 
> `mesos::internal::logging::initialize` without flags.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 62018: Updated logging::initialize to make flags optional.

2017-09-18 Thread Mesos Reviewbot Windows 

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



FAIL: Some Mesos libprocess-tests failed.

Reviews applied: `['62018']`

Failed command: 
`C:\mesos\3rdparty\libprocess\src\tests\Debug\libprocess-tests.exe`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62018

Relevant logs:

- 
[libprocess-tests-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62018/logs/libprocess-tests-stdout.log):

```
[ RUN  ] FutureTest.ArrowOperator
[   OK ] FutureTest.ArrowOperator (0 ms)
[ RUN  ] FutureTest.UndiscardableFuture
[   OK ] FutureTest.UndiscardableFuture (0 ms)
[ RUN  ] FutureTest.UndiscardableLambda
[   OK ] FutureTest.UndiscardableLambda (1 ms)
[--] 17 tests from FutureTest (378 ms total)

[--] 1 test from HTTPTest
[ RUN  ] HTTPTest.PipeReadAll
[   OK ] HTTPTest.PipeReadAll (2 ms)
[--] 1 test from HTTPTest (21 ms total)

[--] 8 tests from HTTPConnectionTest
[ RUN  ] HTTPConnectionTest.GzipRequestBody
[   OK ] HTTPConnectionTest.GzipRequestBody (28 ms)
[ RUN  ] HTTPConnectionTest.Serial
[   OK ] HTTPConnectionTest.Serial (36 ms)
[ RUN  ] HTTPConnectionTest.Pipeline
[   OK ] HTTPConnectionTest.Pipeline (61 ms)
[ RUN  ] HTTPConnectionTest.ClosingRequest
[   OK ] HTTPConnectionTest.ClosingRequest (31 ms)
[ RUN  ] HTTPConnectionTest.ClosingResponse
[   OK ] HTTPConnectionTest.ClosingResponse (28 ms)
[ RUN  ] HTTPConnectionTest.ReferenceCounting
[   OK ] HTTPConnectionTest.ReferenceCounting (5 ms)
[ RUN  ] HTTPConnectionTest.Equality
[   OK ] HTTPConnectionTest.Equality (7 ms)
[ RUN  ] HTTPConnectionTest.RequestStreaming
```

- Mesos Reviewbot Windows


On Sept. 18, 2017, 1:05 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62018/
> ---
> 
> (Updated Sept. 18, 2017, 1:05 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-7586
> https://issues.apache.org/jira/browse/MESOS-7586
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This will be used to initialize logging in the main functions
> that do not use `mesos::internal::logging::Flags`.
> 
> 
> Diffs
> -
> 
>   src/logging/logging.hpp 0e448f25de6c66d772f5bc481fbaf4aa493fd8af 
>   src/logging/logging.cpp 70d66a5c396f709e8f27ad0d51315ed6d257f73b 
> 
> 
> Diff: https://reviews.apache.org/r/62018/diff/4/
> 
> 
> Testing
> ---
> 
> ```
> $ make check
> ```
> 
> Also tested manually to see if using `LOG()` without initializing GLOG gives 
> the same result as using it after running 
> `mesos::internal::logging::initialize` without flags.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 62018: Updated logging::initialize to make flags optional.

2017-09-18 Thread Armand Grillet 

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

(Updated Sept. 18, 2017, 1:05 p.m.)


Review request for mesos, Andrei Budnik and Alexander Rukletsov.


Bugs: MESOS-7586
https://issues.apache.org/jira/browse/MESOS-7586


Repository: mesos


Description (updated)
---

This will be used to initialize logging in the main functions
that do not use `mesos::internal::logging::Flags`.


Diffs
-

  src/logging/logging.hpp 0e448f25de6c66d772f5bc481fbaf4aa493fd8af 
  src/logging/logging.cpp 70d66a5c396f709e8f27ad0d51315ed6d257f73b 


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


Testing (updated)
---

```
$ make check
```

Also tested manually to see if using `LOG()` without initializing GLOG gives 
the same result as using it after running 
`mesos::internal::logging::initialize` without flags.


Thanks,

Armand Grillet

Re: Review Request 62018: Updated logging::initialize to make flags optional.

2017-09-14 Thread Andrei Budnik 

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


Ship it!




Ship It!

- Andrei Budnik


On Sept. 14, 2017, 8:34 a.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62018/
> ---
> 
> (Updated Sept. 14, 2017, 8:34 a.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-7586
> https://issues.apache.org/jira/browse/MESOS-7586
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Thiw will be used to initialize logging in the main functions
> that do not use mesos::internal::logging::Flags.
> 
> 
> Diffs
> -
> 
>   src/logging/logging.hpp 0e448f25de6c66d772f5bc481fbaf4aa493fd8af 
>   src/logging/logging.cpp 70d66a5c396f709e8f27ad0d51315ed6d257f73b 
> 
> 
> Diff: https://reviews.apache.org/r/62018/diff/4/
> 
> 
> Testing
> ---
> 
> ```
> $ make check
> ```
> 
> Also tested manually to see if using `LOG()` without initializing GLOG gives 
> the same result as using it `LOG()` after running 
> `mesos::internal::logging::initialize` without flags.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 62018: Updated logging::initialize to make flags optional.

2017-09-14 Thread Armand Grillet 

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

(Updated Sept. 14, 2017, 8:34 a.m.)


Review request for mesos, Andrei Budnik and Alexander Rukletsov.


Changes
---

Removed helper.


Bugs: MESOS-7586
https://issues.apache.org/jira/browse/MESOS-7586


Repository: mesos


Description
---

Thiw will be used to initialize logging in the main functions
that do not use mesos::internal::logging::Flags.


Diffs (updated)
-

  src/logging/logging.hpp 0e448f25de6c66d772f5bc481fbaf4aa493fd8af 
  src/logging/logging.cpp 70d66a5c396f709e8f27ad0d51315ed6d257f73b 


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

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


Testing
---

```
$ make check
```

Also tested manually to see if using `LOG()` without initializing GLOG gives 
the same result as using it `LOG()` after running 
`mesos::internal::logging::initialize` without flags.


Thanks,

Armand Grillet

Re: Review Request 62018: Updated logging::initialize to make flags optional.

2017-09-12 Thread Armand Grillet 

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

(Updated Sept. 12, 2017, 11:33 a.m.)


Review request for mesos, Andrei Budnik and Alexander Rukletsov.


Changes
---

Resolved issue.


Bugs: MESOS-7586
https://issues.apache.org/jira/browse/MESOS-7586


Repository: mesos


Description
---

Thiw will be used to initialize logging in the main functions
that do not use mesos::internal::logging::Flags.


Diffs (updated)
-

  src/logging/logging.hpp 0e448f25de6c66d772f5bc481fbaf4aa493fd8af 
  src/logging/logging.cpp 70d66a5c396f709e8f27ad0d51315ed6d257f73b 


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

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


Testing (updated)
---

```
$ make check
```

Also tested manually to see if using `LOG()` without initializing GLOG gives 
the same result as using it `LOG()` after running 
`mesos::internal::logging::initialize` without flags.


Thanks,

Armand Grillet

Re: Review Request 62018: Updated logging::initialize to make flags optional.

2017-09-12 Thread Armand Grillet 


> On Sept. 4, 2017, 1:39 p.m., Alexander Rukletsov wrote:
> > src/logging/logging.cpp
> > Lines 135-139 (patched)
> > 
> >
> > This approach changes defaults for clients, who start calling 
> > `logging::initialize(argv0)` now. For example, according to 
> > https://github.com/google/glog/blob/v0.3.3/src/logging.cc#L140,
> > the default for `FLAGS_logbufsecs` will change from `30` to `0`. More 
> > importantly, the default for `FLAGS_logtostderr` is `false` (see 
> > https://github.com/google/glog/blob/v0.3.3/src/logging.cc#L101), while with 
> > your change it will become `true`.
> > 
> > It is hard to say whether it is an issue or not (probably not). If you 
> > want to code defensively, you should guard parts of this function with `if 
> > (_flags.isSome()) {...}`. However, I think it's fine to keep it as is, 
> > extending the comment here saying that glog defaults can be overridden.

The new patch adds guard parts.


- Armand


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


On Sept. 6, 2017, 9:48 a.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62018/
> ---
> 
> (Updated Sept. 6, 2017, 9:48 a.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-7586
> https://issues.apache.org/jira/browse/MESOS-7586
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Thiw will be used to initialize logging in the main functions
> that do not use mesos::internal::logging::Flags.
> 
> 
> Diffs
> -
> 
>   src/logging/logging.hpp 0e448f25de6c66d772f5bc481fbaf4aa493fd8af 
>   src/logging/logging.cpp 70d66a5c396f709e8f27ad0d51315ed6d257f73b 
> 
> 
> Diff: https://reviews.apache.org/r/62018/diff/2/
> 
> 
> Testing
> ---
> 
> ```
> $ make check
> ```
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 62018: Updated logging::initialize to make flags optional.

2017-09-11 Thread Armand Grillet 


> On Sept. 8, 2017, 12:30 p.m., Andrei Budnik wrote:
> > src/logging/logging.cpp
> > Line 156 (original), 165 (patched)
> > 
> >
> > What will be the value of `FLAGS_logtostderr` if `_flags` is `None`? 
> > Does it mean that messages via `LOG` won't be written to `stdout`/`stderr`?

By default, `FLAGS_logtostderr` value is 0. Thus we will not log to stderr 
instead of log files.


- Armand


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


On Sept. 6, 2017, 9:48 a.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62018/
> ---
> 
> (Updated Sept. 6, 2017, 9:48 a.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-7586
> https://issues.apache.org/jira/browse/MESOS-7586
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Thiw will be used to initialize logging in the main functions
> that do not use mesos::internal::logging::Flags.
> 
> 
> Diffs
> -
> 
>   src/logging/logging.hpp 0e448f25de6c66d772f5bc481fbaf4aa493fd8af 
>   src/logging/logging.cpp 70d66a5c396f709e8f27ad0d51315ed6d257f73b 
> 
> 
> Diff: https://reviews.apache.org/r/62018/diff/2/
> 
> 
> Testing
> ---
> 
> ```
> $ make check
> ```
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 62018: Updated logging::initialize to make flags optional.

2017-09-08 Thread Andrei Budnik 

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




src/logging/logging.cpp
Line 156 (original), 165 (patched)


What will be the value of `FLAGS_logtostderr` if `_flags` is `None`? Does 
it mean that messages via `LOG` won't be written to `stdout`/`stderr`?


- Andrei Budnik


On Sept. 6, 2017, 9:48 a.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62018/
> ---
> 
> (Updated Sept. 6, 2017, 9:48 a.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-7586
> https://issues.apache.org/jira/browse/MESOS-7586
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Thiw will be used to initialize logging in the main functions
> that do not use mesos::internal::logging::Flags.
> 
> 
> Diffs
> -
> 
>   src/logging/logging.hpp 0e448f25de6c66d772f5bc481fbaf4aa493fd8af 
>   src/logging/logging.cpp 70d66a5c396f709e8f27ad0d51315ed6d257f73b 
> 
> 
> Diff: https://reviews.apache.org/r/62018/diff/2/
> 
> 
> Testing
> ---
> 
> ```
> $ make check
> ```
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 62018: Updated logging::initialize to make flags optional.

2017-09-06 Thread Armand Grillet 

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

(Updated Sept. 6, 2017, 9:48 a.m.)


Review request for mesos, Andrei Budnik and Alexander Rukletsov.


Changes
---

Resolved issues.


Bugs: MESOS-7586
https://issues.apache.org/jira/browse/MESOS-7586


Repository: mesos


Description (updated)
---

Thiw will be used to initialize logging in the main functions
that do not use mesos::internal::logging::Flags.


Diffs (updated)
-

  src/logging/logging.hpp 0e448f25de6c66d772f5bc481fbaf4aa493fd8af 
  src/logging/logging.cpp 70d66a5c396f709e8f27ad0d51315ed6d257f73b 


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

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


Testing
---

```
$ make check
```


Thanks,

Armand Grillet

Re: Review Request 62018: Updated logging::initialize to make flags optional.

2017-09-04 Thread Alexander Rukletsov 

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




src/logging/logging.hpp
Lines 31-32 (original), 31-32 (patched)


Please sync var names with the definition.



src/logging/logging.cpp
Lines 135-139 (patched)


This approach changes defaults for clients, who start calling 
`logging::initialize(argv0)` now. For example, according to 
https://github.com/google/glog/blob/v0.3.3/src/logging.cc#L140,
the default for `FLAGS_logbufsecs` will change from `30` to `0`. More 
importantly, the default for `FLAGS_logtostderr` is `false` (see 
https://github.com/google/glog/blob/v0.3.3/src/logging.cc#L101), while with 
your change it will become `true`.

It is hard to say whether it is an issue or not (probably not). If you want 
to code defensively, you should guard parts of this function with `if 
(_flags.isSome()) {...}`. However, I think it's fine to keep it as is, 
extending the comment here saying that glog defaults can be overridden.



src/logging/logging.cpp
Line 138 (original), 144 (patched)


We haven't initialized glog yet. Please log to `stderr` and exit instead.



src/logging/logging.cpp
Line 149 (original), 155 (patched)


Ditto.


- Alexander Rukletsov


On Aug. 31, 2017, 4:47 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62018/
> ---
> 
> (Updated Aug. 31, 2017, 4:47 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-7586
> https://issues.apache.org/jira/browse/MESOS-7586
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This will be used to initialize logging in the main functions
> that do not use mesos::internal::logging::Flags.
> 
> 
> Diffs
> -
> 
>   src/logging/logging.hpp 0e448f25de6c66d772f5bc481fbaf4aa493fd8af 
>   src/logging/logging.cpp 70d66a5c396f709e8f27ad0d51315ed6d257f73b 
> 
> 
> Diff: https://reviews.apache.org/r/62018/diff/1/
> 
> 
> Testing
> ---
> 
> ```
> $ make check
> ```
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 62018: Updated logging::initialize to make flags optional.

2017-08-31 Thread Mesos Reviewbot Windows 

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



Bad patch!

Reviews applied: [62018]

Logs available here: http://104.210.40.105/logs/master/62018

- Mesos Reviewbot Windows


On Aug. 31, 2017, 4:47 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62018/
> ---
> 
> (Updated Aug. 31, 2017, 4:47 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-7586
> https://issues.apache.org/jira/browse/MESOS-7586
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This will be used to initialize logging in the main functions
> that do not use mesos::internal::logging::Flags.
> 
> 
> Diffs
> -
> 
>   src/logging/logging.hpp 0e448f25de6c66d772f5bc481fbaf4aa493fd8af 
>   src/logging/logging.cpp 70d66a5c396f709e8f27ad0d51315ed6d257f73b 
> 
> 
> Diff: https://reviews.apache.org/r/62018/diff/1/
> 
> 
> Testing
> ---
> 
> ```
> $ make check
> ```
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>