voxpupuli / puppet-staging

⛔️ Deprecated in favor of puppet-archive
https://forge.puppet.com/puppet/archive
Apache License 2.0
51 stars 109 forks source link

Implement a create_target parameter #44

Closed JCotton1123 closed 10 years ago

nanliu commented 10 years ago

This is the responsibility of the consumer of this module, because the caller is the only one that knows how many directory it needs to create the necessary target directory, so I don't think it make sense to manage it here.

The one thing it adds value is to establish this relationship:

before => Exec["extract ${name}"]

I would really recommend using puppet-archive instead because it can handle these things via types/provider much better: https://github.com/nanliu/puppet-archive/blob/master/lib/puppet/type/archive.rb#L153

JCotton1123 commented 10 years ago

On Mon, Oct 20, 2014 at 7:20 PM, Nan Liu notifications@github.com wrote:

This is the responsibility of the consumer of this module, because the caller is the only one that knows how many directory it needs to create the necessary target directory, so I don't think it make sense to manage it here.

I understand why you're saying this however I added it to address a specific use case. Most often when I use strip-component I want to place the content in a specific known directory. This makes that possible without having to add logic somewhere else. I'll also note that this is helpful when working with WARs b/c the content is often not nested within a folder.

If you'd prefer I can replace the file resource with an exec "mkdir -p ${target}" but this becomes a bit messy when trying to deal with the user/group permissions.

The one thing it adds value is to establish this relationship:

before => Exec["extract ${name}"]

I would really recommend using puppet-archive instead because it can handle these things via types/provider much better:

https://github.com/nanliu/puppet-archive/blob/master/lib/puppet/type/archive.rb#L153

I looked at this but I believe it suffers from the same "problem". We're already using the staging module and I'd rather not switch to another module.

This feature is off by default and it doesn't add any complexity to the module. Please reconsider accepting the PR.

Reply to this email directly or view it on GitHub https://github.com/nanliu/puppet-staging/pull/44#issuecomment-59855937.

nanliu commented 10 years ago

@JCotton1123 I'm not suggesting mkdir -p, but rather this whole thing is better for the consumer of this module instead of this module. This new feature doesn't really work for nested dir, and it can cause duplicate resources. So, rather than implementing it here, if you feel a define type is suitable for your environment, you can create a custom define wrapper and replace staging::deploy:

acme::deploy (
...
$create_target =
) {
  if $create_target {
    file { $target:
...

HTH,

Nan