Jira (PUP-7640) Property::List is inconsistent with respect to #is_to_s and #should_to_s

1 view
Skip to first unread message

Thomas Hallgren (JIRA)

unread,
Jun 9, 2017, 2:43:03 AM6/9/17
to puppe...@googlegroups.com
Thomas Hallgren created an issue
 
Puppet / Bug PUP-7640
Property::List is inconsistent with respect to #is_to_s and #should_to_s
Issue Type: Bug Bug
Assignee: Unassigned
Created: 2017/06/08 11:42 PM
Priority: Normal Normal
Reporter: Thomas Hallgren

Problem 1.
The Puppet::Property::List class, super class of Puppet::Property::OrderedList implements the method #is_to_s but does not implement #should_to_s. This leads to inconsistent representation of the current and to values when change events are reported.

The groups property of a User type can for instance present changes like:

Notice: /Stage[main]/Main/User[foobar]/groups: groups changed wheel,sshd to ['mail', 'sshd', 'wheel']

Problem 2.
Since the delimiter separated list is no longer quoted by default due to the changes in PUP-7616, a list with no entries is represented as an empty string. Adding the first group to a user looks like this:

Notice: /Stage[main]/Main/User[foobar]/groups: groups changed  to ['wheel']

The methods #is_to_s and #should_to_s should be implemented the same way and we need to make a decision on the representation.

Alternative 1.
The property is changed to rely on default behavior (i.e. the special #is_to_s is simply removed. The result will then be that both values are represented as bracketed arrays and that the current delimiter property is rendered useless. The above changes would then instead look like:

groups changed ['sshd', 'wheel'] to ['mail', 'sshd', 'wheel']
groups changed [] to ['wheel']

Alternative 2.
A #should_to_s is added that is equal to #is_to_s. Both methods present a list of entries joined by the given delimiter. This list must then be quoted to avoid problem #2. The changes would then be presented as:

groups changed 'sshd,wheel' to 'mail,sshd,wheel'
groups changed '' to 'wheel'

While this alternative vouches for a more compact presentation, it is also problematic. List is abstract and used as the parent of by many properties. The presentation will look strange if one or several entries contain:

  1. the delimiter
  2. a single quote
  3. a complex object
    The presentation is also different from how the property is presented by commands like puppet resource, which always uses the bracketed array.
Add Comment Add Comment
 
This message was sent by Atlassian JIRA (v6.4.14#64029-sha1:ae256fe)
Atlassian logo

Thomas Hallgren (JIRA)

unread,
Jun 9, 2017, 2:44:02 AM6/9/17
to puppe...@googlegroups.com
Thomas Hallgren updated an issue
Change By: Thomas Hallgren
*Problem 1.*
The {{Puppet::Property::List}} class, super class of {{Puppet::Property::OrderedList}} implements the method {{#is_to_s}} but does not implement {{#should_to_s}}. This leads to inconsistent representation of the _current_ and _to_ values when change events are reported.

The _groups_ property of a {{User}} type can for instance present changes like:
{code}

Notice: /Stage[main]/Main/User[foobar]/groups: groups changed wheel,sshd to ['mail', 'sshd', 'wheel']
{code}

*Problem 2.*

Since the delimiter separated list is no longer quoted by default due to the changes in PUP-7616, a list with no entries is represented as an empty string. Adding the first group to a user looks like this:
{code}

Notice: /Stage[main]/Main/User[foobar]/groups: groups changed  to ['wheel']
{code}

The methods
 {{  #is_to_s }}  and  {{  #should_to_s }}  should be implemented the same way and we need to make a decision on the representation.

*Alternative 1.*
The property is changed to rely on default behavior (i.e. the special {{#is_to_s}} is simply removed. The result will then be that both values are represented as bracketed arrays and that the current _delimiter_ property is rendered useless. The above changes would then instead look like:
{code}

groups changed ['sshd', 'wheel'] to ['mail', 'sshd', 'wheel']
groups changed [] to ['wheel']
{code}

*Alternative 2.*
A {{#should_to_s}} is added that is equal to {{#is_to_s}}. Both methods present a list of entries joined by the given delimiter. This list must then be quoted to avoid problem #2. The changes would then be presented as:
{code}

groups changed 'sshd,wheel' to 'mail,sshd,wheel'
groups changed '' to 'wheel'
{code}

While this alternative vouches for a more compact presentation, it is also problematic. {{List}} is abstract and used as the parent of by many properties. The presentation will look strange if one or several entries contain:
# the delimiter
# a single quote
# a complex object
The presentation is also different from how the property is presented by commands like {{puppet resource}}, which always uses the bracketed array.

Thomas Hallgren (JIRA)

unread,
Jun 9, 2017, 2:46:02 AM6/9/17
to puppe...@googlegroups.com

Thomas Hallgren (JIRA)

unread,
Jun 9, 2017, 2:47:02 AM6/9/17
to puppe...@googlegroups.com
Thomas Hallgren updated an issue
*Problem 1.*
The {{Puppet::Property::List}} class, super class of {{Puppet::Property::OrderedList}} implements the method {{#is_to_s}} but does not implement {{#should_to_s}}. This leads to inconsistent representation of the _current_ and _to_ values when change events are reported.

The _groups_ property of a {{User}} type can for instance present changes like:
{code}
Notice: /Stage[main]/Main/User[foobar]/groups: groups changed wheel,sshd to ['mail', 'sshd', 'wheel']
{code}

*Problem 2.*
Since the delimiter separated list is no longer quoted by default due to the changes in PUP-7616,  consequently,  a list with no entries is represented as an empty string. Adding the first group to a user looks like this:

Josh Cooper (JIRA)

unread,
Jun 23, 2017, 6:48:03 PM6/23/17
to puppe...@googlegroups.com

Josh Cooper (Jira)

unread,
Jun 14, 2021, 3:20:01 PM6/14/21
to puppe...@googlegroups.com
Josh Cooper commented on Bug PUP-7640
 
Re: Property::List is inconsistent with respect to #is_to_s and #should_to_s

We should go with Alternative 1, so that the output matches how one would specify the values in a manifest:

user { 'bob':
  ensure => present,
  groups => ['users', 'wheel']
}

This message was sent by Atlassian Jira (v8.13.2#813002-sha1:c495a97)
Atlassian logo
Reply all
Reply to author
Forward
0 new messages