Re: Review Request 43911: Updated `/state` agent endpoint to use jsonify.

[Michael Park]

> On March 10, 2016, 7 p.m., Michael Park wrote:
> > src/slave/http.cpp, line 105
> > 
> >
> > If we were using `model` before, we need to maintain that.
> > 
> > We have a `json` defined for `CommandInfo` in `src/common/http.cpp`, we 
> > should add the declaration `void json(JSON::ObjectWriter* writer, const 
> > CommandInfo& command);` to `src/common/http.hpp`.
> > 
> > We can then use it like this:
> > 
> > ```
> > writer->field("command", task.command());
> > ```

This does not seem to have been addressed.


- Michael


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


On March 17, 2016, 9:33 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43911/
> ---
> 
> (Updated March 17, 2016, 9:33 p.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated `/state` agent endpoint to use jsonify.
> 
> 
> Diffs
> -
> 
>   src/slave/http.cpp 4eb1fafdfa72094511b0b2684a3c2705bd8c7c5e 
> 
> Diff: https://reviews.apache.org/r/43911/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 43911: Updated `/state` agent endpoint to use jsonify.

[Michael Park]

> On March 24, 2016, 4:38 p.m., Michael Park wrote:
> > src/slave/http.cpp, line 96
> > 
> >
> > This should be marked `static`, right?

This is addressed in https://reviews.apache.org/r/44981/.


- Michael


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


On March 17, 2016, 9:33 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43911/
> ---
> 
> (Updated March 17, 2016, 9:33 p.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated `/state` agent endpoint to use jsonify.
> 
> 
> Diffs
> -
> 
>   src/slave/http.cpp 4eb1fafdfa72094511b0b2684a3c2705bd8c7c5e 
> 
> Diff: https://reviews.apache.org/r/43911/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 43911: Updated `/state` agent endpoint to use jsonify.

[Neil Conway]

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

(Updated March 17, 2016, 9:33 p.m.)


Review request for mesos and Michael Park.


Changes
---

Fix output for slave's `queuedTasks`.


Repository: mesos


Description
---

Updated `/state` agent endpoint to use jsonify.


Diffs (updated)
-

  src/slave/http.cpp 4eb1fafdfa72094511b0b2684a3c2705bd8c7c5e 

Diff: https://reviews.apache.org/r/43911/diff/


Testing
---

make check


Thanks,

Neil Conway

Re: Review Request 43911: Updated `/state` agent endpoint to use jsonify.

[Neil Conway]

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

(Updated March 16, 2016, 9:14 p.m.)


Review request for mesos and Michael Park.


Changes
---

Rebase.


Repository: mesos


Description
---

Updated `/state` agent endpoint to use jsonify.


Diffs (updated)
-

  src/slave/http.cpp 4eb1fafdfa72094511b0b2684a3c2705bd8c7c5e 

Diff: https://reviews.apache.org/r/43911/diff/


Testing
---

make check


Thanks,

Neil Conway

Re: Review Request 43911: Updated `/state` agent endpoint to use jsonify.

[Neil Conway]

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

(Updated March 11, 2016, 10:33 p.m.)


Review request for mesos and Michael Park.


Changes
---

Address code review.


Repository: mesos


Description
---

Updated `/state` agent endpoint to use jsonify.


Diffs (updated)
-

  src/common/http.hpp 61c63a09c428331f7561a9c4c0254bf4e1c8915f 
  src/common/http.cpp 3e92979567c7bcefa9ab5f32aa7b23000aa9deb8 
  src/slave/http.cpp 4eb1fafdfa72094511b0b2684a3c2705bd8c7c5e 
  src/tests/common/http_tests.cpp d1bafeb2c5298f9eda8f3927d4ff2ad62b1bb0be 

Diff: https://reviews.apache.org/r/43911/diff/


Testing
---

make check


Thanks,

Neil Conway

Re: Review Request 43911: Updated `/state` agent endpoint to use jsonify.

[Michael Park]

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




src/slave/http.cpp (lines 89 - 105)


This needs to be defined in the namespace that `TaskInfo` is defined, that 
is, in the `mesos` namespace.



src/slave/http.cpp (line 97)


If we were using `model` before, we need to maintain that.

We have a `json` defined for `CommandInfo` in `src/common/http.cpp`, we 
should add the declaration `void json(JSON::ObjectWriter* writer, const 
CommandInfo& command);` to `src/common/http.hpp`.

We can then use it like this:

```
writer->field("command", task.command());
```



src/slave/http.cpp (line 125)


Same as above, since we were using `model` before, we need to maintain 
this. We just need to remove the `JSON::Protobuf` here.

```
writer->element(task);
```



src/slave/http.cpp (line 493)


Curious as to why we added a newline here.



src/slave/http.cpp (line 497)


Same here, why the newline?


- Michael Park


On Feb. 29, 2016, 7:01 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43911/
> ---
> 
> (Updated Feb. 29, 2016, 7:01 a.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated `/state` agent endpoint to use jsonify.
> 
> 
> Diffs
> -
> 
>   src/slave/http.cpp 4eb1fafdfa72094511b0b2684a3c2705bd8c7c5e 
> 
> Diff: https://reviews.apache.org/r/43911/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 43911: Updated `/state` agent endpoint to use jsonify.

[Neil Conway]

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

(Updated Feb. 29, 2016, 7:01 a.m.)


Review request for mesos and Michael Park.


Changes
---

Revert unintended change.


Repository: mesos


Description
---

Updated `/state` agent endpoint to use jsonify.


Diffs (updated)
-

  src/slave/http.cpp 4eb1fafdfa72094511b0b2684a3c2705bd8c7c5e 

Diff: https://reviews.apache.org/r/43911/diff/


Testing
---

make check


Thanks,

Neil Conway

Re: Review Request 43911: Updated `/state` agent endpoint to use jsonify.

[Neil Conway]

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

(Updated Feb. 29, 2016, 6:55 a.m.)


Review request for mesos and Michael Park.


Changes
---

Rebase.


Repository: mesos


Description
---

Updated `/state` agent endpoint to use jsonify.


Diffs (updated)
-

  src/slave/http.cpp 4eb1fafdfa72094511b0b2684a3c2705bd8c7c5e 

Diff: https://reviews.apache.org/r/43911/diff/


Testing
---

make check


Thanks,

Neil Conway

Re: Review Request 43911: Updated `/state` agent endpoint to use jsonify.

[Neil Conway]

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

(Updated Feb. 28, 2016, 12:16 a.m.)


Review request for mesos and Michael Park.


Changes
---

Rebase.


Repository: mesos


Description
---

Updated `/state` agent endpoint to use jsonify.


Diffs (updated)
-

  src/slave/http.cpp 4eb1fafdfa72094511b0b2684a3c2705bd8c7c5e 

Diff: https://reviews.apache.org/r/43911/diff/


Testing
---

make check


Thanks,

Neil Conway

Re: Review Request 43911: Updated `/state` agent endpoint to use jsonify.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [43816, 43822, 43823, 43817, 43910, 43911]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On Feb. 23, 2016, 10:29 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43911/
> ---
> 
> (Updated Feb. 23, 2016, 10:29 p.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated `/state` agent endpoint to use jsonify.
> 
> 
> Diffs
> -
> 
>   src/slave/http.cpp a18085ea020d0d6c39f23213e11af75a02eedb7e 
> 
> Diff: https://reviews.apache.org/r/43911/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Review Request 43911: Updated `/state` agent endpoint to use jsonify.

[Neil Conway]

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

Review request for mesos and Michael Park.


Repository: mesos


Description
---

Updated `/state` agent endpoint to use jsonify.


Diffs
-

  src/slave/http.cpp a18085ea020d0d6c39f23213e11af75a02eedb7e 

Diff: https://reviews.apache.org/r/43911/diff/


Testing
---

make check


Thanks,

Neil Conway