mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Benjamin Mahler <bmah...@apache.org>
Subject Re: Review Request 61109: Used the default value when parsing an optional enum field.
Date Sat, 09 Sep 2017 00:53:00 GMT


> On Sept. 8, 2017, 11:59 p.m., James Peach wrote:
> > This looks pretty reasonable to me. It's unfortunate that this will convert all
invalid enum names into the default value, but AFAICT that is unavoidable.

Since we're talking about optional enums, it's not obvious to me whether it's better to leave
it unset or to set it to the default. With a required enum, we can't leave it unset so it
seems like the default value makes the most sense. However, shouldn't the caller specify the
behavior they want? Much like `JsonParseOptions.ignore_unknown_fields` is an explicit option?
This would be something like `use_default_for_unknown_enum_values`?


- Benjamin


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


On July 25, 2017, 3:17 p.m., Qian Zhang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61109/
> -----------------------------------------------------------
> 
> (Updated July 25, 2017, 3:17 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and James Peach.
> 
> 
> Bugs: MESOS-7828
>     https://issues.apache.org/jira/browse/MESOS-7828
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Used the default value when parsing an optional enum field.
> 
> 
> Diffs
> -----
> 
>   3rdparty/stout/include/stout/protobuf.hpp 15690b66cc4ae0c1bf2c2176d73c385ca75d3c20

> 
> 
> Diff: https://reviews.apache.org/r/61109/diff/1/
> 
> 
> Testing
> -------
> 
> With this patch, when accessing master endpoint with an inexistent enum `xxx` in a JSON:
> ```
> curl -X POST -H "Content-Type: application/json" -d '{"type": "xxx"}' 127.0.0.1:5050/api/v1
> ```
> The master log will be:
> ```
> I0725 23:09:53.097790   665 http.cpp:1133] HTTP POST for /master/api/v1 from 127.0.0.1:49566
with User-Agent='curl/7.47.0'
> I0725 23:09:53.098006   665 http.cpp:669] Processing call UNKNOWN
> ```
> This proves when parsing an inexistent enum the default enum value (i.e., `UNKNOWN`)
will be used.
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>


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