On Mon, Feb 13, 2017 at 5:27 PM, Benjamin Mahler <bmah...@apache.org> wrote:
> The way I think about this is that if the field is semantically required
>
"semantically required": good point and this should definitely be one of
the criteria.
I guess we still need to clarify this for each of these messages: there's
another class of "semantically required" fields that you DO need to set we
need to disambiguate.
I'll start by improving the comments on `Filters`.
> then there can be a default. If not, then the optionality has meaning. For
> example, if there is always a notion of filtering, then having a default
> filter makes sense. But if the absence of a filter means no filtering
> occurs, then absence of the optional field has a meaning and we don't
> interpret the overall message to have a default value.
>
> Also, if we want to move to proto3 syntax at some point, we'll have to push
> our defaults into our API handling code rather than in the proto file
> AFAICT.
>
> On Thu, Feb 2, 2017 at 12:06 PM, Yan Xu <xuj...@apple.com> wrote:
>
> > With protobuf you can specify custom default values for scalar types
> > (proto2 at least) but not message types, e.g.,
> >
> > ```
> > message Filters {
> > // Time to consider unused resources refused. Note that all unused
> > // resources will be considered refused and use the default value
> > // (below) regardless of whether Filters was passed to
> > // SchedulerDriver::launchTasks. You MUST pass Filters with this
> > // field set to change this behavior (i.e., get another offer which
> > // includes unused resources sooner or later than the default).
> > optional double refuse_seconds = 1 [default = 5.0];
> > }
> > ```
> >
> > However, the message `Filters` essential has a default value because
> *all*
> > its
> > fields have default values. It all depends on whether the receiver
> chooses
> > to check it is not set or directly accesses it and gets the default
> values.
> >
> > When we reference the type in other messages, e.g.,
> >
> > ```
> > message Accept {
> > repeated OfferID offer_ids = 1;
> > repeated Offer.Operation operations = 2;
> > optional Filters filters = 3;
> > }
> > ```
> >
> > We are not explicitly telling users what's going to happen when `filters`
> > is not set. The master just directly uses it without checking.
> >
> > It does feel intuitive to me that "*if all the fields in a message have
> > default values, and it semantically feels like a config, then we can just
> > interpret them when unset as indication to use defaults*".
> >
> > However we probably should document it better.
> >
> > To generalize it further, for something like this with multiple fields
> >
> > ```
> > message ExponentialBackoff {
> > optional double initial_interval_seconds = 1 [default = 0.5];
> > optional double max_interval_seconds = 2 [default = 300.0];
> > optional double randomization_factor = 3 [default = 0.5];
> > optional double max_elapsed_seconds = 4 [default = 2592000.0];
> > }
> > ```
> >
> > we should be able to not require them to be set and assume the defaults?
> >
> > One step further, if the message has recursively nested messages with
> > default values, we can treat the parent message as having a default value
> > too?
> >
> > Thoughts?
> >
> > Yan
> >
>
