REEF-968 Handle RequiredParameter set to null

["Dudoladov, Sergey" <se...@tu-berlin.de>]

Hi all,

The original intent of REEF-968 was to reset the RequiredParameter to the default value, if the configuration attempts to set the parameter to null. 

Now I am not so sure that silently using the default value is a good policy, and I think it is better to fail fast with the exception.  The relevant code in ConfigurationModule already raises the exception with a meaningful message, so I suggest we close the REEF-968 issue as "won't fix". 


Thanks to Minhyeok Kweun for bringing attention to the issue.

Best,
Sergey.

Re: REEF-968 Handle RequiredParameter set to null

["Dudoladov, Sergey" <se...@tu-berlin.de>]

> All hail Tang's pedantic error reporting for avoiding a bug :)

Let the God bless Tang !

Re: REEF-968 Handle RequiredParameter set to null

[Markus Weimer <ma...@weimo.de>]

On Mon, Aug 15, 2016 at 10:16 AM, Tobin Baker <td...@cs.washington.edu> wrote:
> Silently substituting a default value would have masked a bug in our configuration code.

All hail Tang's pedantic error reporting for avoiding a bug :)

Markus

Re: REEF-968 Handle RequiredParameter set to null

[Tobin Baker <td...@cs.washington.edu>]

As the user who originally found this bug, I agree :) Silently substituting
a default value would have masked a bug in our configuration code.

On Wed, Aug 10, 2016 at 2:13 PM, Markus Weimer <ma...@weimo.de> wrote:

> On 2016-08-09 13:35, Dudoladov, Sergey wrote:
>
>> The original intent of REEF-968 was to reset the RequiredParameter to
>> the default value, if the configuration attempts to set the parameter
>> to null.
>>
>> Now I am not so sure that silently using the default value is a good
>> policy, and I think it is better to fail fast with the exception.
>> The relevant code in ConfigurationModule already raises the exception
>> with a meaningful message, so I suggest we close the REEF-968 issue
>> as "won't fix".
>>
>
> +1 If the user willfully sets a parameter to `null` where `null` is not an
> acceptable value, we should raise an exception indeed.
>
> Markus
>
>

Re: REEF-968 Handle RequiredParameter set to null

[Markus Weimer <ma...@weimo.de>]

On 2016-08-09 13:35, Dudoladov, Sergey wrote:
> The original intent of REEF-968 was to reset the RequiredParameter to
> the default value, if the configuration attempts to set the parameter
> to null.
>
> Now I am not so sure that silently using the default value is a good
> policy, and I think it is better to fail fast with the exception.
> The relevant code in ConfigurationModule already raises the exception
> with a meaningful message, so I suggest we close the REEF-968 issue
> as "won't fix".

+1 If the user willfully sets a parameter to `null` where `null` is not 
an acceptable value, we should raise an exception indeed.

Markus