Some boolean properties were being ignored when false.

[orium]

For instance, if you set durable = false it would not define durable which would default to true.

[wyardley]

This makes some sense to me; I have a few questions, though:

• Does this affect other providers also?
• Are you using false or 'false' in your config, and is your config in hiera or in a Puppet manifest?
  for example:
  https://github.com/voxpupuli/puppet-rabbitmq/blob/v8.2.2/spec/unit/puppet/provider/rabbitmq_exchange/rabbitmqadmin_spec.rb#L48-L57 -- note that the booleans are quoted here
• Should we be doing a bool to string conversion rather than changing the if resource[:foo] bit?
• Can you provide a config snippet that creates this condition?

[orium]

Hi @wyardley

Does this affect other providers also?

No, they don't seem to use this kind of logic.

Are you using false or 'false' in your config

I was using false (without quotes), which explain why it didn't work. In any case, a boolean should be supported (well, in fact it should be a boolean and not a string).

your config in hiera or in a Puppet manifest?

hiera.

Should we be doing a bool to string conversion rather than changing the if resource[:foo] bit?

Even if we do, the if resource[:foo] should not be there anyway (they have default values of 'false').

The if resource[:foo] would make sense, however, if someone explicitly puts a nil there. Do we care about that?

Can you provide a config snippet that creates this condition?

profile_rabbitmq::exchanges
  'foo-notifications-e@/':
    durable: false
    type: 'direct'

[wyardley]

I’m not totally sure about that; seeing if @bastelfreak has any thoughts
I mostly agree with you that supporting both string and Boolean makes sense here, but I think we have sometimes required people to use the string of true / false when those are used as actual values for settings. I’m not sure if there would be a valid use case for nil here, kind of assuming not, but not totally sure.

[orium]

I’m not totally sure about that

About what in particular?

[wyardley]

I’m not totally sure about that

About what in particular?

Sorry, I think I meant in response to your question about whether nil is a valid use case here.

[wyardley]

https://github.com/voxpupuli/puppet-rabbitmq/blob/v8.2.2/lib/puppet/provider/rabbitmq_queue/rabbitmqadmin.rb#L85 would be kind of similar situation, I think (and doesn't have that guard clause).

I'll wait for someone else to weigh in, and will do a quick test on a vagrant box, but I'm +1 on this.

[wyardley]

They also seem to default to 'false', and have a regex check that they're not empty, so I guess nil is probably not ever valid. On the other hand, I would have thought defaultto: 'false' should have prevented this from being a problem if the parameter were not set at all (vs. being a boolean false).

https://github.com/voxpupuli/puppet-rabbitmq/blob/master/lib/puppet/type/rabbitmq_exchange.rb#L40-L44

[wyardley]

Tested this locally, and it does seem to work as expected.
With the changes from this PR, the below code works as expected whether durable is false or 'false'; without them, a boolean false causes that item to get omitted.

      rabbitmq_exchange { 'exchange2@host2':
        user     => 'dan',
        password => 'bar',
        type     => 'topic',
        durable  => false,
        ensure   => present,
      }

[orium]

Is it possible to do a release including this fix?

[bastelfreak]

I plan a new release as soon as #716 got merged.

[orium]

Awesome. thanks.