RFC: Make file content specification methods consistent.

[Nigel Kersten]

http://projects.puppetlabs.com/issues/5158

----------------------- Ticket description ---------------

We have four main ways we can specify file content in a file resource.

The source parameter
The content parameter
The file function
The template function

These behave inconsistently in the following ways.

The source parameter, file function and template function all can take
an array. For source/file, the first file that exists will be used.
For the template function, we concatenate the templates instead.

The file function takes fully qualified paths only. The template
function takes fully qualified paths, or dereferences relative paths
as follows. ‘foo/bar.erb’ –> modules/foo/templates/bar.erb

The latter problem is relatively easily solved, particularly if we
implement #4885

We are going to have to break backwards compatibility to solve the
first problem however.

My feeling is that more people make use of the multi-select logic in
the source parameter/file function than make use of the concatenation
of the template function.
-----------------------

I'm opening this up for discussion here on the user list as we need to
all agree whether it's worth chasing consistency here at the cost of
breaking backwards compatibility.

It appears that people use both the concatenation and multi-select
logic. How can we provide both bits of functionality for all these
methods?

Here's a terrible suggestion that hopefully inspires a better one.
An array indicates multi-select logic, separation with a colon means
concatenate.

1a. Use the first source that exists.

file { "/tmp/somefile":
source => ["puppet:///modules/foo/somefile.$hostname",
"puppet:///modules/foo/somefile.default",]
}

file { "/tmp/somefile":
content => template("foo/somefile.$hostname.erb",
"foo/somefile.default.erb"),
}

1b. Concatenate multiple objects

file { "/tmp/somefile":
source => "puppet:///modules/foo/somefile.$hostname:puppet:///modules/foo/somefile.default",
}

file { "/tmp/somefile":
content => template("foo/somefile.$hostname.erb:foo/somefile.default.erb"),
}

Is this so unsatisfactory that we need to add more parameters? What if
we pluralized for the concatenation with "sources" and "contents" ?

2b. New parameter for concatenation.

file { "/tmp/somefile":
sources => ["puppet:///modules/foo/somefile.$hostname",
"puppet:///modules/foo/somefile.default",]
}

file { "/tmp/somefile":
contents => [template("foo/somefile.$hostname.erb",
template("foo/somefile.default.erb")],
}

Alternatively, do we really need to fix this? I think we do, as
consistency matters a lot to me, but maybe I'm on my own here?

--
Nigel Kersten - Puppet Labs -  http://www.puppetlabs.com

[Patrick]

The best solution I can come up with is this.

source - Unchanged because this is used so much. Tries each source in turn and uses the first one that works. Fails if none work.
content - Deprecated but kept for backwards compatibility
source_concat - Concatenates all the given sources and causes the resource to fail if any are missing.
content_concat - behaves the same way as content does now and takes precedence over content.

Throw an error if two or more are used at the same time, except allow content and content_concat to exist at the same time for backwards compatibility.

I also think the inconsistencies between "file()" and "template()" should be addressed, but this would simplify it.

[Nigel Kersten]

Would it be better if we could make the template function be aware of
a boolean parameter "concatenate" that changed the behavior when an
array is passed to it? (and we do the same thing for source)

> I also think the inconsistencies between "file()" and "template()" should be addressed, but this would simplify it.

Absolutely. I would prefer to leave the existing behavior around for a
while and we implement http://projects.puppetlabs.com/issues/4885
which I think was your suggestion anyway :)

>
> --
> You received this message because you are subscribed to the Google Groups "Puppet Users" group.
> To post to this group, send email to puppet...@googlegroups.com.
> To unsubscribe from this group, send email to puppet-users...@googlegroups.com.
> For more options, visit this group at http://groups.google.com/group/puppet-users?hl=en.

[Patrick]

Sorry but you totally lost me. Does the template() function even take an array now?

>> I also think the inconsistencies between "file()" and "template()" should be addressed, but this would simplify it.
>
> Absolutely. I would prefer to leave the existing behavior around for a
> while and we implement http://projects.puppetlabs.com/issues/4885
> which I think was your suggestion anyway :)

Mostly. I was just saying, lets brainstorm about the parameters here and worry about the functions somewhere else.

[Nigel Kersten]

That would be because I said "array" instead of "multiple arguments",
which is what I meant, but after sketching it out, I don't see any
significant advantage over your suggestion of {source,content}_concat.

>>> I also think the inconsistencies between "file()" and "template()" should be addressed, but this would simplify it.
>>
>> Absolutely. I would prefer to leave the existing behavior around for a
>> while and we implement http://projects.puppetlabs.com/issues/4885
>> which I think was your suggestion anyway :)
>
> Mostly.  I was just saying, lets brainstorm about the parameters here and worry about the functions somewhere else.

We can do that as far as making template() and file() behave the same
way, but we can't ignore the functions altogether, as the
inconsistency between template() and source => is the major problem
here.

We don't have to rush to find a solution. This would be a big change
that we wouldn't push out in a point release.

[Al @ Lab42]

IMHO both the alternatives are OK, and, referring to the post's
followups, I prefer something like sources to source_concat and would
avoid the use of a "concatenate" boolean parameter to influence the
behaviour of another parameter (source/content): better to have the
information of how files are provided in a single parameter.

But basically it's just a matter of aestetics.

> Alternatively, do we really need to fix this? I think we do, as
> consistency matters a lot to me, but maybe I'm on my own here

Not at all, you're right, this has to be fixed.

My c
Al

[Marc Zampetti]

I would avoid the use of a plural version of a parameter. To easy to
make a mistake and since it is valid won't be easy to diagnose. The
_concat would be fine. Also, isn't there already a parameter that
influences the behavior if the "source" parameter? I don't recall the
exact name, something like sourceselect I think. How would that factor
into all of this?

[Simon Deziel]

On 10/30/2010 11:45 AM, Nigel Kersten wrote:
> http://projects.puppetlabs.com/issues/5158
>
> ----------------------- Ticket description ---------------
>
> We have four main ways we can specify file content in a file resource.
>
>
> The source parameter
> The content parameter
> The file function
> The template function
>
> These behave inconsistently in the following ways.
>
> The source parameter, file function and template function all can take
> an array. For source/file, the first file that exists will be used.
> For the template function, we concatenate the templates instead.
>
> The file function takes fully qualified paths only. The template
> function takes fully qualified paths, or dereferences relative paths

> as follows. �foo/bar.erb� �> modules/foo/templates/bar.erb

>
> The latter problem is relatively easily solved, particularly if we
> implement #4885
>
> We are going to have to break backwards compatibility to solve the
> first problem however.
>
> My feeling is that more people make use of the multi-select logic in
> the source parameter/file function than make use of the concatenation
> of the template function.
> -----------------------
>
> I'm opening this up for discussion here on the user list as we need to
> all agree whether it's worth chasing consistency here at the cost of
> breaking backwards compatibility.
>
> It appears that people use both the concatenation and multi-select
> logic. How can we provide both bits of functionality for all these
> methods?
>
> Here's a terrible suggestion that hopefully inspires a better one.
> An array indicates multi-select logic, separation with a colon means
> concatenate.

I would prefer to use " + " as the separator to indicate concatenation.

> 1a. Use the first source that exists.
>
> file { "/tmp/somefile":
> source => ["puppet:///modules/foo/somefile.$hostname",
> "puppet:///modules/foo/somefile.default",]
> }
>
> file { "/tmp/somefile":
> content => template("foo/somefile.$hostname.erb",
> "foo/somefile.default.erb"),
> }
>
> 1b. Concatenate multiple objects
>
> file { "/tmp/somefile":
> source => "puppet:///modules/foo/somefile.$hostname:puppet:///modules/foo/somefile.default",
> }
>
> file { "/tmp/somefile":
> content => template("foo/somefile.$hostname.erb:foo/somefile.default.erb"),
> }
>
> Is this so unsatisfactory that we need to add more parameters? What if
> we pluralized for the concatenation with "sources" and "contents" ?

IMHO, the use of the plural form makes it error prone.

> 2b. New parameter for concatenation.
>
> file { "/tmp/somefile":
> sources => ["puppet:///modules/foo/somefile.$hostname",
> "puppet:///modules/foo/somefile.default",]
> }
>
> file { "/tmp/somefile":
> contents => [template("foo/somefile.$hostname.erb",
> template("foo/somefile.default.erb")],
> }
>
> Alternatively, do we really need to fix this? I think we do, as
> consistency matters a lot to me, but maybe I'm on my own here?

I agree with you that consistency is of great concern and this should be
addressed.

[Trevor Hemsley]

Simon Deziel wrote:
> I would prefer to use " + " as the separator to indicate concatenation.
>

+1

This sounds like the ideal solution to me and clearly indicates that the
strings are to be concatenated.

--

Trevor Hemsley
Infrastructure Engineer
.................................................
* C A L Y P S O
* 4th Floor, Tower Point,
44 North Road,
Brighton, BN1 1YR, UK

OFFICE +44 (0) 1273 666 350
FAX +44 (0) 1273 666 351

.................................................
www.calypso.com

This electronic-mail might contain confidential information intended
only for the use by the entity named. If the reader of this message is
not the intended recipient, the reader is hereby notified that any
dissemination, distribution or copying is strictly prohibited.

* P * /*/ Please consider the environment before printing this e-mail /*/

[Daniel Pittman]

Nigel Kersten <ni...@puppetlabs.com> writes:

> ----------------------- Ticket description ---------------
>
> We have four main ways we can specify file content in a file resource.
>
> The source parameter
> The content parameter
> The file function
> The template function

[...]

> Here's a terrible suggestion that hopefully inspires a better one. An array
> indicates multi-select logic, separation with a colon means concatenate.

That is terrible, not least because it makes use of a valid filename component
that some platforms (*cough* Macintosh *cough*) have used as a vital part of
their path descriptions, and others use routinely.

Hint: /etc/environment/default-path and content is now gonna suck. :)

> 1a. Use the first source that exists.

[...]

> Is this so unsatisfactory that we need to add more parameters?

I think it could be satisfactory with only a tiny change that will *also*
solve another end-user problem we recently saw on the list:

file { "/etc/environment/default-path":
content => "/bin:/usr/bin:/usr/local/bin" + $more_path
}

source => "puppet:///foo/bar" + "http://example.com/data"

Oh, and this just works(tm):

source => ["puppet:///foo/bar.${fqdn}", "puppet:///foo/bar"] +
# OK, now I am just makin' stuff up at random. ;)
["erb:///foo/bar.$fqdn.erb", "template"///foo/bar.erb"] +
# ...or is that actually nicer than this API change?
# (selection, not concatenation)
template("foo/bar.$fqdn.erb", "foo/bar.erb")

Other string concatenation character choices would work, but perhaps would be
suboptimal from a being-consistent-with-Ruby point of view, if that matters.
(Also, functions, but this is likely to be common enough to justify an
operator in the circumstances.)[1]

The internal implementation may be complicated by this, somewhat, but a
sufficiently smart implementation should be possible that makes it just
work(tm) as the use would expect in the long term.[2]

This also, incidentally, fixes the implicit wish from a recent poster who
wanted to perform a lookup on a log filename based on a set of concatenated
values or so. (Actually, I think he wanted interpolated variable names, but
whatever. Maybe it wasn't relevant after all.)

> What if we pluralized for the concatenation with "sources" and "contents" ?

I agree with other posters that the distinction is accurate, but probably
error-prone; it would be easy to miss that when reviewing code, or have people
wonder if it was just a typo and have their manifests fail...

[...]

> Alternatively, do we really need to fix this? I think we do, as consistency
> matters a lot to me, but maybe I'm on my own here?

No, I think it does improve things measurably to make this consistent and
predictable. Nothing makes it harder to use a language than utterly unbounded
complexity explosions, but this sort of inconsistency comes a close second.

Daniel

Footnotes:
[1] ...or does that cause a conflict between math and concatenation when we
say something like 'if $x + $y > 12 { ... }' then?

[2] Specifically, I can see a potential conflict between the concatenation
operator and the idea of fetching HTTP URLs rather than just puppet://
ones, based on when that fetch happens - master or client.

Which is a worry that requires puppet gaining the ability to fetch
arbitrary content anyway; without that added there isn't much new
complexity here.

--
✣ Daniel Pittman ✉ dan...@rimspace.net ☎ +61 401 155 707
♽ made with 100 percent post-consumer electrons

[Patrick]

On Oct 31, 2010, at 11:18 PM, Daniel Pittman wrote:

I think it could be satisfactory with only a tiny change that will *also*
solve another end-user problem we recently saw on the list:

 file { "/etc/environment/default-path":
   content => "/bin:/usr/bin:/usr/local/bin" + $more_path
 }

 source => "puppet:///foo/bar" + "http://example.com/data"

To me this says, add the strings together then use that as the source.  I would assume "add" means concatenate which means puppet would look for a file at:

protocol: puppet

path: "foo/barhttp://example.com/data"

So this doesn't work for me.

Also, this requires changing the grammar of he puppet parser which I think shouldn't be done to fix one resource.

Oh, and this just works(tm):

 source => ["puppet:///foo/bar.${fqdn}", "puppet:///foo/bar"] +
           # OK, now I am just makin' stuff up at random. ;)
           ["erb:///foo/bar.$fqdn.erb", "template"///foo/bar.erb"] +
           # ...or is that actually nicer than this API change?
           # (selection, not concatenation)
           template("foo/bar.$fqdn.erb", "foo/bar.erb")

The erb protocol stuff in here sounds to me like http://projects.puppetlabs.com/issues/4920 which is very hard to do for technical reasons.  It's mostly because the variables used in the erb file are not sent to the file server.

Note: The template() function is executed on the server when the manifest is compiled.

On 10/30/2010 11:45 AM, Nigel Kersten wrote:

http://projects.puppetlabs.com/issues/5158

>The source parameter, file function and template function all can take an array. For source/file, the first file that exists will be used. For the template function, we concatenate the templates instead.

>The file function takes fully qualified paths only. The template function takes fully qualified paths, or dereferences relative paths as follows. ‘foo/bar.erb’ –> modules/foo/templates/bar.erb

>The latter problem is relatively easily solved, particularly if we implement #4885

>We are going to have to break backwards compatibility to solve the first problem however.

But you don't with my earlier suggestion quoted here:

On Oct 30, 2010, at 9:46 AM, Patrick wrote:

The best solution I can come up with is this.

source - Unchanged because this is used so much.  Tries each source in turn and uses the first one that works.  Fails if none work.
content - Deprecated but kept for backwards compatibility
source_concat - Concatenates all the given sources and causes the resource to fail if any are missing.
content_concat - behaves the same way as content does now and takes precedence over content.

Throw an error if two or more are used at the same time, except allow content and content_concat to exist at the same time for backwards compatibility.

I also think the inconsistencies between "file()" and "template()" should be addressed, but this would simplify it.

This only changes the provider.  Most of the suggestions I've heard change the server and the language.  I think this also keeps full backward compatibility.  (If it doesn't, please tell me)  You say you don't want to add another parameter, but adding another parameter is better than hacking the whole language for one resource.

Also, having the provider parse very complicated input causes a lot of confusion and bugs.  (If you don't believe this, look at the Augeas provider and it's quoting nightmare.)  This way also doesn't reserve any characters in the source string.  Instead it uses the existing parser to handle (almost) all the parsing.

[Nigel Kersten]

Doesn't this fail to offer the find-first-existing feature of 'source'
for 'content' and thus leaves us in an inconsistent state again?

I really don't think we can deprecate 'content' altogether, as while
we're here trying to decide how to make these various chunks of
functionality consistent, we also have to keep in mind the common use
case of simply specifying 'content' as a string.

Do you really want to deprecate 'content' for such cases and force
people into using 'content_concat' ?

> On Oct 30, 2010, at 9:46 AM, Patrick wrote:
>
> The best solution I can come up with is this.
>
> source - Unchanged because this is used so much.  Tries each source in turn
> and uses the first one that works.  Fails if none work.
> content - Deprecated but kept for backwards compatibility
> source_concat - Concatenates all the given sources and causes the resource
> to fail if any are missing.
> content_concat - behaves the same way as content does now and takes
> precedence over content.
>
> Throw an error if two or more are used at the same time, except allow
> content and content_concat to exist at the same time for backwards
> compatibility.
>
>
> I also think the inconsistencies between "file()" and "template()" should be
> addressed, but this would simplify it.
>
> This only changes the provider.  Most of the suggestions I've heard change
> the server and the language.  I think this also keeps full backward
> compatibility.  (If it doesn't, please tell me)  You say you don't want to
> add another parameter, but adding another parameter is better than hacking
> the whole language for one resource.
> Also, having the provider parse very complicated input causes a lot of
> confusion and bugs.  (If you don't believe this, look at the Augeas provider
> and it's quoting nightmare.)  This way also doesn't reserve any characters
> in the source string.  Instead it uses the existing parser to handle
> (almost) all the parsing.
>

> --
> You received this message because you are subscribed to the Google Groups
> "Puppet Users" group.
> To post to this group, send email to puppet...@googlegroups.com.
> To unsubscribe from this group, send email to
> puppet-users...@googlegroups.com.
> For more options, visit this group at
> http://groups.google.com/group/puppet-users?hl=en.
>

--

[jcbollinger]

On Oct 30, 11:46 am, Patrick <kc7...@gmail.com> wrote:

> The best solution I can come up with is this.
>
> source - Unchanged because this is used so much.  Tries each source in turn and uses the first one that works.  Fails if none work.
> content - Deprecated but kept for backwards compatibility
> source_concat - Concatenates all the given sources and causes the resource to fail if any are missing.
> content_concat - behaves the same way as content does now and takes precedence over content.
>
> Throw an error if two or more are used at the same time, except allow content and content_concat to exist at the same time for backwards compatibility.

1) Why add a new property "content_concat" when equivalent
functionality could be made more broadly available by adding add a
string concatenation function or operator by which to compose the
value from its constituent pieces? E.g.:

content => concat("# Standard Header\n", $my_content_piece1,
inline_template('<%= ... %>'))

No new parameter and no deprecation is needed, and there can be 100%
backwards compatibility with respect to this parameter.

2) I'm not so keen on source_concat either, though I like it better
than content_concat. If there is concatenation to be done, then
should it not be done on the puppetmaster? Among other things, only
that way can a checksum be computed. Checksumming the individual
pieces doesn't work because the client does not have separate pieces
to check. If, then, the puppetmaster is going to compose pieces and
checksum them, why should it not serve the composed result? And in
that case, what would differentiate the source_concat property from
the content (or content_concat) property?

In short, I think the current "content" and "source" parameters both
work fine exactly as currently designed, and indeed, both are designed
as they are for good reason. Furthermore, I think it is nicely
consistent that once the relative path issue is fixed for file(),

content => file("foo", "bar")

will produce the same client-side content as

source => ["foo", 'bar"]


Consider now that it's template() that is the oddball here. Given a
concat() function as suggested above, however,

template("t1", "t2")

would be equivalent to

concat( template("t1"), template("t2") )

so how about resolving the perceived inconsistency by deprecating
passing multiple arguments to template()? The feature need not be
removed any time soon (indeed, it might never be removed), but the
replacement would be both more expressive and trivially consistent
with File/source and file().


To sum up:

Resolving the perceived consistency issue can be done without any
change to the File resource. It is sufficient to

a) Fix the relative path problem for the file() function, and
b) add a string concatentation function, and
c) deprecate passing multiple arguments to template(), prefering
instead applying the new conatentation function to several single-
argument invocations of template().

This approach is 100% backwards-compatible, inasmuch as deprecation of
multiple arguments to template() does not imply removal of that
feature. Because the new features would all be in functions, they
would be available everywhere in manifests that functions can be
invoked, not just in File resources. Furthermore, the new mechanism
for concatenating template results would be both more expressive than
the old and, because of its reliance on a general-purpose function,
more broadly consistent than just with file() and File/source.


Regards,

John

[Richard Crowley]

(This is probably too radical but bear with me.)

The proposals floating around involving some functions, some +
operators, and some parameters that accept strings or arrays seem to
create lots of leaky abstractions.

For example, if the + operator does anything but concatenate strings
or add numbers, I as a user would be surprised and confused.
Dereferencing its argument pathnames into content seems far outside
its scope. (Patrick's reply above is spot on and more detailed on
this front.)

Similarly, if a style parameter or the like indicated whether the
multi-select or concatenation behavior were in effect and actually
controlled the behavior of function calls like template() and file(),
I'd be surprised and confused (and probably angry). Those function
calls should behave the same way in any context.

Here comes the radical part: what if we deprecated the content
parameter, the template() function, and the file() function? In their
place we add parameters called template and file (maybe a better name
for that one). These, along with the source parameter, accept a
string that is resolved to a pathname or an array that resolves to
multiple pathnames. The style (for lack of a better name) parameter
chooses multi-select or concatenation behavior. For example:

file {
"/foo":
template => "module/foo";
"/bar":
source => ["puppet:///module/bar", "puppet:///module/bar2"],
style => select;
"/baz":
file => ["module/baz-header", "module/baz-body",
"module/baz-footer"],
style => concat;
}

There is precedent for one parameter affecting the behavior of
another: exec's path and cwd parameters have a good deal to say about
what the named command actually does. Precedent aside, I don't think
it's surprising to users that the parameters given to a resource work
together to satisfy it.

It surely wouldn't be easy to get rid of the content parameter, and
the template() and file() functions but I think it's worth a
discussion.

Rich

[Michael Gliwinski]

On Monday 01 Nov 2010 15:28:42 jcbollinger wrote:
> o sum up:
>
> Resolving the perceived consistency issue can be done without any
> change to the File resource. It is sufficient to
>
> a) Fix the relative path problem for the file() function, and
> b) add a string concatentation function, and
> c) deprecate passing multiple arguments to template(), prefering
> instead applying the new conatentation function to several single-
> argument invocations of template().
>
> This approach is 100% backwards-compatible, inasmuch as deprecation of
> multiple arguments to template() does not imply removal of that
> feature. Because the new features would all be in functions, they
> would be available everywhere in manifests that functions can be
> invoked, not just in File resources. Furthermore, the new mechanism
> for concatenating template results would be both more expressive than
> the old and, because of its reliance on a general-purpose function,
> more broadly consistent than just with file() and File/source.

+1

Just had the same thoughts :D

--
Michael Gliwinski
Henderson Group Information Services
9-11 Hightown Avenue, Newtownabby, BT36 4RT
Phone: 028 9034 3319

**********************************************************************************************
The information in this email is confidential and may be legally privileged. It is intended solely for the addressee and access to the email by anyone else is unauthorised.
If you are not the intended recipient, any disclosure, copying, distribution or any action taken or omitted to be taken in reliance on it, is prohibited and may be unlawful.
When addressed to our clients, any opinions or advice contained in this e-mail are subject to the terms and conditions expressed in the governing client engagement leter or contract.
If you have received this email in error please notify sup...@henderson-group.com

John Henderson (Holdings) Ltd
Registered office: 9 Hightown Avenue, Mallusk, County Antrim, Northern Ireland, BT36 4RT.
Registered in Northern Ireland
Registration Number NI010588
Vat No.: 814 6399 12
*********************************************************************************

[Patrick]

On Nov 1, 2010, at 7:31 AM, Nigel Kersten wrote:

On Mon, Nov 1, 2010 at 1:21 AM, Patrick <kc7...@gmail.com> wrote:
I really don't think we can deprecate 'content' altogether, as while
we're here trying to decide how to make these various chunks of
functionality consistent, we also have to keep in mind the common use
case of simply specifying 'content' as a string.

Do you really want to deprecate 'content' for such cases and force
people into using 'content_concat' ?

I'm going with jcbollinger's solution instead, but content_concat would have done the same thing when passed one argument as content.

On Nov 1, 2010, at 8:28 AM, jcbollinger wrote:

1) Why add a new property "content_concat" when equivalent
functionality could be made more broadly available by adding add a
string concatenation function or operator by which to compose the
value from its constituent pieces?  E.g.:

content => concat("# Standard Header\n", $my_content_piece1,
inline_template('<%= ... %>'))

No new parameter and no deprecation is needed, and there can be 100%
backwards compatibility with respect to this parameter.

2) If there is concatenation to be done, then

should it not be done on the puppetmaster?  Among other things, only
that way can a checksum be computed.  Checksumming the individual
pieces doesn't work because the client does not have separate pieces
to check.  If, then, the puppetmaster is going to compose pieces and
checksum them, why should it not serve the composed result?  And in
that case, what would differentiate the source_concat property from
the content (or content_concat) property?

I had totally missed that problem.

To sum up:

Resolving the perceived consistency issue can be done without any
change to the File resource.  It is sufficient to

a) Fix the relative path problem for the file() function, and
b) add a string concatentation function, and
c) deprecate passing multiple arguments to template(), prefering
instead applying the new conatentation function to several single-
argument invocations of template().

This approach is 100% backwards-compatible, inasmuch as deprecation of
multiple arguments to template() does not imply removal of that
feature.  Because the new features would all be in functions, they
would be available everywhere in manifests that functions can be
invoked, not just in File resources.  Furthermore, the new mechanism
for concatenating template results would be both more expressive than
the old and, because of its reliance on a general-purpose function,
more broadly consistent than just with file() and File/source.

+1

[Brice Figureau]

On 01/11/10 16:28, jcbollinger wrote:
[ snipped full proposal ]

>
> To sum up:
>
> Resolving the perceived consistency issue can be done without any
> change to the File resource. It is sufficient to
>
> a) Fix the relative path problem for the file() function, and
> b) add a string concatentation function, and
> c) deprecate passing multiple arguments to template(), prefering
> instead applying the new conatentation function to several single-
> argument invocations of template().
>
> This approach is 100% backwards-compatible, inasmuch as deprecation of
> multiple arguments to template() does not imply removal of that
> feature. Because the new features would all be in functions, they
> would be available everywhere in manifests that functions can be
> invoked, not just in File resources. Furthermore, the new mechanism
> for concatenating template results would be both more expressive than
> the old and, because of its reliance on a general-purpose function,
> more broadly consistent than just with file() and File/source.

+1, this is I think the best, and more or less what I was thinking when
I read the OP mail.

One thing I'd really don't want to lose (and some of the previous
proposition removed it) is the hability to use:

content => "my content"

Thanks,
--
Brice Figureau
My Blog: http://www.masterzen.fr/

[Nigel Kersten]

On Mon, Nov 1, 2010 at 5:56 PM, Brice Figureau
<brice-...@daysofwonder.com> wrote:
> On 01/11/10 16:28, jcbollinger wrote:
> [ snipped full proposal ]
>>
>> To sum up:
>>
>> Resolving the perceived consistency issue can be done without any
>> change to the File resource.  It is sufficient to
>>
>> a) Fix the relative path problem for the file() function, and
>> b) add a string concatentation function, and
>> c) deprecate passing multiple arguments to template(), prefering
>> instead applying the new conatentation function to several single-
>> argument invocations of template().
>>
>> This approach is 100% backwards-compatible, inasmuch as deprecation of
>> multiple arguments to template() does not imply removal of that
>> feature.  Because the new features would all be in functions, they
>> would be available everywhere in manifests that functions can be
>> invoked, not just in File resources.  Furthermore, the new mechanism
>> for concatenating template results would be both more expressive than
>> the old and, because of its reliance on a general-purpose function,
>> more broadly consistent than just with file() and File/source.
>
> +1, this is I think the best, and more or less what I was thinking when
> I read the OP mail.

I'm happy with the direction this is going in.

It would be great if someone could update the original bug with the
position we've come to, otherwise I'll get to it in the next few days.

>
> One thing I'd really don't want to lose (and some of the previous
> proposition removed it) is the hability to use:
>
> content => "my content"
>
> Thanks,
> --
> Brice Figureau
> My Blog: http://www.masterzen.fr/
>

> --
> You received this message because you are subscribed to the Google Groups "Puppet Users" group.
> To post to this group, send email to puppet...@googlegroups.com.
> To unsubscribe from this group, send email to puppet-users...@googlegroups.com.
> For more options, visit this group at http://groups.google.com/group/puppet-users?hl=en.
>
>

--

[jcbollinger]

On Nov 1, 7:07 pm, Nigel Kersten <ni...@puppetlabs.com> wrote:
> It would be great if someone could update the original bug with the
> position we've come to, otherwise I'll get to it in the next few days.

Done.


John

[Nigel Kersten]

Thanks. There's one thing I'm not quite clear on.

This proposal means we have concatenation and find-first-existing
support for the source parameter, and concatenation for the template
function, but how are we proposing we provide find-first-existing
support for the template and file functions? Or are we dropping that
goal?

[Michael Gliwinski]

I believe that was by either passing an array or multiple arguments, no?
E.g.:

find-first-existing:

file('foo', 'bar')

template('foo.erb', 'bar.erb')

concatenation:

concat(file('foo'), file('bar'))
concat(template('foo.erb'), template('bar.erb'))

that way you can even mix them ;)

concat(file('header'), template("foo.$host.erb", "foo.erb"))

[Patrick]

The proposal I was supporting (and the one in the bug tracker) retained backward compatibility which means that passing more than one file to template() still concatenates. That is deprecated though.

*) Would creating a function that says, 'return the first argument that doesn't throw an exception' be useful?
*) Is it even feasible to write?
*) Also, I'm assuming that file() and template() throw an exception if the file doesn't exist. Does anyone know if that's true?
*) Also, what would you name this function?

I'm thinking that if we do take this approach, it should be split off into another ticket.

[Nigel Kersten]

maybe... I'm having trouble thinking of a decent name for this though :)

> *) Also, I'm assuming that file() and template() throw an exception if the file doesn't exist.  Does anyone know if that's true?

From memory they do, but I'll have to double check.

> *) Also, what would you name this function?
>
> I'm thinking that if we do take this approach, it should be split off into another ticket.

I kind of disagree with splitting it off.

Perhaps I didn't express myself well, but my main impetus was to make
the source/file/template data specifications *all* support the
find-first-existing and concatenation functionality in a sane and
consistent manner.

[Brice Figureau]

On 02/11/10 17:43, Nigel Kersten wrote:
> On Tue, Nov 2, 2010 at 4:32 PM, Patrick <kc7...@gmail.com> wrote:
>> *) Would creating a function that says, 'return the first argument
>> that doesn't throw an exception' be useful? *) Is it even feasible
>> to write?
>
> maybe... I'm having trouble thinking of a decent name for this though
> :)

It could be "coalesce" (which is used in SQL for almost the same
function), but since this word also has the meaning of merging, it might
defeat the purpose :)

[Patrick]

I know that was your intent, but I was thinking that from a coding and testing standpoint that these are two independent things.  Because of that, I though it should be split into different related tickets.

[Nigel Kersten]

> I kind of disagree with splitting it off.
>
> Perhaps I didn't express myself well, but my main impetus was to make
> the source/file/template data specifications *all* support the
> find-first-existing and concatenation functionality in a sane and
> consistent manner.
>
> I know that was your intent, but I was thinking that from a coding and
> testing standpoint that these are two independent things.  Because of that,
> I though it should be split into different related tickets.

We're actually sorting out a more rigorous workflow for tickets at the
moment, and we'll be sending out an RFC to the community about the
workflow we'd like to implement once we've finished a minor amount of
bike-shedding internally. This isn't a matter of keeping process
hidden from the community, we just have some internal constraints to
satisfy.

It's not clear what a ticket is meant to capture right now, and we
need to fix that.

[Michael Gliwinski]

On Tuesday 02 Nov 2010 16:43:25 Nigel Kersten wrote:
> >> I believe that was by either passing an array or multiple arguments, no?
> >> E.g.:
> >>
> >> find-first-existing:
> >>
> >> file('foo', 'bar')
> >> template('foo.erb', 'bar.erb')
> >>
> >> concatenation:
> >>
> >> concat(file('foo'), file('bar'))
> >> concat(template('foo.erb'), template('bar.erb'))
> >>
> >> that way you can even mix them ;)
> >>
> >> concat(file('header'), template("foo.$host.erb", "foo.erb"))
> >
> > The proposal I was supporting (and the one in the bug tracker) retained
> > backward compatibility which means that passing more than one file to
> > template() still concatenates. That is deprecated though.
> >
> > *) Would creating a function that says, 'return the first argument that
> > doesn't throw an exception' be useful? *) Is it even feasible to write?
>
> maybe... I'm having trouble thinking of a decent name for this though :)

how about just first() or first_existing() ?

> > *) Also, I'm assuming that file() and template() throw an exception if
> > the file doesn't exist. Does anyone know if that's true?
>
> From memory they do, but I'll have to double check.
>
> > *) Also, what would you name this function?
> >
> > I'm thinking that if we do take this approach, it should be split off
> > into another ticket.
>
> I kind of disagree with splitting it off.
>
> Perhaps I didn't express myself well, but my main impetus was to make

> the source/file/template data specifications all support the

> find-first-existing and concatenation functionality in a sane and
> consistent manner.

Well, I suppose separating both concatenation and find-first-existing
functionalities from source/file()/template() does get you there (flexible and
consitent) at the cost of being little more verbose.

[jcbollinger]

As Patrick already wrote, the proposal that seemed to garner broad
acceptance maintains backwards compatibility by preserving, but
deprecating, template()'s concatentation feature. If the function is
only supposed to be passed one argument, then the question of find-
first does not arise.

If there is a desire for find-first functionality for template(), then
I think the most general solution would be some version of Patrick's
suggested function for determining which file name to use, whether it
be named "coalesce", "first_file", "find_first", "first_existing",
"choose", "options", or something else. Thus:

template( find_first("t1", "t2") )

In that case, you might also consider deprecating multiple arguments
to the file() function in favor of this new approach (i.e.
file( find_first("foo", "bar") ) instead of file("foo", "bar") ).

Alternatively, a variant version of template() could be added that
provides find-first semantics. It could be named template_choose, or
some similar thing. I prefer the other approach, though.


John

[Patrick]

This is what I wanted except reversed. I was assuming that find_first() can't open or find files. It can only look at what it returned to it, so the syntax would be:

find_first( file('foo'), file('bar') )
or
find_first( template('foo'), template('bar') )
or even
find_first( file('foo'), template('bar') )

I think this order gives you more flexibility. Also, you might be able to put a generate() function in there to depending on implementation.

[jcbollinger]

On Nov 3, 11:30 am, Patrick <kc7...@gmail.com> wrote:
> This is what I wanted except reversed.  I was assuming that find_first() can't open or find files.  It can only look at what it returned to it, so the syntax would be:
>
> find_first( file('foo'), file('bar') )
> or
> find_first( template('foo'), template('bar') )
> or even
> find_first( file('foo'), template('bar') )
>
> I think this order gives you more flexibility.  Also, you might be able to put a generate() function in there to depending on implementation.

That would work for me, too. The ability to find_first results from
different kinds of content sources would be nice.

For my suggestion I was assuming that template() and file() don't have
a good way to signal errors. In particular, an empty result cannot be
used for the purpose because it might be the valid and desired
content. If there is a way to make that work then I'd like that
alternative. If not, then I don't see why find_first() would not be
able to find or open files when file() can do.


John

[Patrick]

The biggest problem is that makes those context sensitive because find_first wouldn't know if it should look in the "files" or the "template" folder.  I guess you could:

*) Name those functions differently depending on which it's looking for.

or

*) Have it always look in templates for .erb files and look in files or everything else.