[chef-dev] Re: Re: Re: local file copy resource?

[Daniel DeLeo < >]

Forwarded message:

From: Daniel DeLeo < >
To: Bryan McLellan < >
Date: Monday, December 10, 2012 12:10:43 PM
Subject: Re: [chef-dev] Re: Re: Re: local file copy resource?

On Monday, December 10, 2012 at 10:07 AM, Bryan McLellan wrote:

On Sun, Dec 9, 2012 at 1:44 PM, Jesse Campbell < "> > wrote:

Take a look here:

https://github.com/jcam/chef/commits/expanded_file_resource

You could create pull request for this sort of patch, even if you're

not done. That would allow people to comment in the PR against the

specific parts of the code.

It looks like #action_create no longer differentiates between creating

a file that doesn't exist and updating the content of a file that does

exist. This seems minor, but my gut tells me it is important for Chef

to know the difference so we can be completely honest with the user

about what we did or are going to do, for purposes like whyrun,

auditing, reporting, etc.

I don't think I like overloading the 'create' action to also be a

'copy' action, differing on if a source file exists or not, as we're

not really creating a file.

In #action_move, you remote the source file if it already matches the

destination file. Conceptually this makes sense, but I'd be pretty

surprised if I asked Chef to move a file and it told me that it

deleted it instead.

#copy reports "create a new cookbook_file " but

is used in both Chef::File#action_move and Chef::File#action_create

(when copying a file). Again, we need to be accurate in what we're

reporting.

Personally the 'to' attribute made more sense to me than 'source.'

When I think about what I'm copying or what I'm moving, I think what I

am describing is the source, not the destination. But I can totally

see how this is possibly personal preference. We see this in

remote_file, cookbook_file and template today. The resource manages

the destination and there is a source attribute. In link, where we

already deal with two files on disk, we use 'to' since we realized

this made more sense in CHEF-30 [1].

Bryan

[1] http://tickets.opscode.com/browse/CHEF-30

Oops, I failed at reply-all sending this originally :/

 

I'm against merging all of these behaviors into a single resource/provider, until there's a better understanding of what we're trying to do, what the related impact will be and what we'll do to mitigate that.

In particular, one problem we've had is where a base kind of resource (service, package, etc.) has fields (like the options field for packages) that are only supported by/relevant to some of the providers it uses. Users then expect these fields to modify behavior in a certain way, when they are in fact silently ignored. The problem also goes the other way; for example, one can create a package resource and set the provider to GemPackage, but then you can't specify all of the relevant parameters that the gem provider expects/respects. The first case could be addressed with validation, but would be more complicated than necessary, because the validation logic needs to know which provider would be used and which options that provider implements.

In this particular case, we're adding a bunch of options that could potentially be combined in invalid ways and also result in very different behaviors. At the same time, we're only going half way: will template be rolled into file? remote file? What about remote_directory and directory?

Bryan made the point that which resource to use should be determined by the kind of thing you're managing (file in this case), but we can't completely divorce what you're managing and how you manage it in the resource declaration. A hypothetical do-everything file resource would need to support cookbook/template name, local template switch and path, content parameter, URL of source, and (now) path of source. These are all mutually exclusive, so we need to catch this and return a sane error to the user if, say, they accidentally leave one parameter in there when changing the resource to be backed by cookbook file to being a template. The actual back-end implementation of all of these things is also quite different, so ideally there would also be some sort of dynamic provider switching based on the attributes present so that the provider doesn't devolve into a mess of case statements based on the selected mode of managing the file.

Despite my reservations above, I'm not opposed to unifying similar resources if we're convinced it makes chef cleaner or easier to use, but I'm also not currently convinced we aren't just moving complexity around and creating a bunch of work in terms of user education and doc changes for what might be a bike shed issue. If reality really is the first case, then we need to make sure we come up with an end result that handles edge cases and invalid input which are now caught by NoMethodError and the like.

-- 
Daniel DeLeo