aurora-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Mehrdad Nurolahzade <mehr...@nurolahzade.com>
Subject Re: Review Request 50052: AURORA-1736 Display reservations and persistent volumes in /offers debug http endpoint
Date Mon, 18 Jul 2016 19:32:08 GMT


> On July 17, 2016, 3:15 p.m., Stephan Erb wrote:
> > Should we consider adopting `protobuf-java-util`'s JsonFormatter rather than implementing
this on our own? 
> > 
> > * https://github.com/google/protobuf/blob/master/java/util/src/main/java/com/google/protobuf/util/JsonFormat.java
> > * http://mvnrepository.com/artifact/com.google.protobuf/protobuf-java-util/3.0.0-beta-3
> 
> Mehrdad Nurolahzade wrote:
>     Looking into protobuf util ...
> 
> Mehrdad Nurolahzade wrote:
>     I can see that we already have [jackson-datatype-protobuf](https://github.com/HubSpot/jackson-datatype-protobuf)
library in the project that can be used to create a JSON serializer from protobuf. We can
probably get this task done using it. However, ```protobuf-java-util``` seems to have a larger
and more active community.

I spent some time trying to get ```protobuf-java-util``` to work. However, it seems to a very
light-weight utility designed to creat one-to-one mapping from protobuf to JSON. Controls
to exclude or give special treatment to fields in the mapping are non-existant which makes
the resulting generated JSON look significantly different from what we are serving today.

Then I looked into ```jackson-datatype-protobuf``` and I found a great degree of flexibility
in it to control generated JSON that I did not see in ```protobuf-java-util```. To generate
JSON output:

```
  @GET
  @Produces(MediaType.APPLICATION_JSON)
  public Response getOffers() throws JsonProcessingException {
    ObjectMapper mapper = new ObjectMapper()
        .registerModule(new ProtobufModule())
        .setPropertyNamingStrategy(PropertyNamingStrategy.CAMEL_CASE_TO_LOWER_CASE_WITH_UNDERSCORES)
        .setSerializationInclusion(JsonInclude.Include.NON_NULL);
    return Response.ok(mapper.writeValueAsString(offerManager.getOffers())).build();
  }
```

However, the resulting JSON output is different from what we used to serve (see the attachments).
If this change is acceptable (and won't break anything else) I can use it as-is. Otherwise,
I have to write code to customize the output (filter out certain fields, change rendering,
etc.) which I feel somehow defeats the purpose of this refactoring.


- Mehrdad


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


On July 15, 2016, 1:15 p.m., Mehrdad Nurolahzade wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50052/
> -----------------------------------------------------------
> 
> (Updated July 15, 2016, 1:15 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Stephan Erb.
> 
> 
> Bugs: AURORA-1736
>     https://issues.apache.org/jira/browse/AURORA-1736
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> AURORA-1736 Display reservations and persistent volumes in /offers debug http endpoint
> 
> 
> Diffs
> -----
> 
>   config/legacy_untested_classes.txt 1ea2183ab20cc5c6bca147bcea4e5c708d576b62 
>   src/main/java/org/apache/aurora/scheduler/http/Offers.java 80f082410896a50d86c7886736caf79581f5051c

>   src/test/java/org/apache/aurora/scheduler/http/OffersTest.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/50052/diff/
> 
> 
> Testing
> -------
> 
> Manual, Jenkins, and end_to_end
> 
> 
> Thanks,
> 
> Mehrdad Nurolahzade
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message