I haven't really delved into this patch, but it looks like you're  
mostly cleaning up the fileserver code and dealing well with missing  
files, right?

Comments below.

On Sep 17, 2008, at 10:40 AM, Paul Nasrat wrote:

>
>
> Signed-off-by: Paul Nasrat <[EMAIL PROTECTED]>
> ---
> lib/puppet/network/handler/fileserver.rb |   25 +++++++++++++ 
> +-----------
> spec/unit/type/file.rb                   |   17 ++++++++++-------
> 2 files changed, 24 insertions(+), 18 deletions(-)
>
> diff --git a/lib/puppet/network/handler/fileserver.rb b/lib/puppet/ 
> network/handler/fileserver.rb
> index 3e62cdb..6bd6f2f 100755
> --- a/lib/puppet/network/handler/fileserver.rb
> +++ b/lib/puppet/network/handler/fileserver.rb
> @@ -656,30 +656,25 @@ class Puppet::Network::Handler
>
>             # Recursively list the files in this tree.
>             def reclist(basepath, abspath, recurse, ignore)

Isn't the problem really, as you mentioned, that the :links setting  
isn't propagating?  If so, doesn't it make sense to just expand the  
prototype here?

It might also be worth investigating the use of the Fileset class from  
the new file_serving code.  It's much simpler and much cleaner, and it  
might be straightforward to just replace the existing code with it.   
If you look at that, you're likely to need to backport a few patches  
from master.  Or rather, from the code I'm about to merge into master  
(the file_serving over REST code).
>
> -            @catalog.add_resource @copy_resource
> +            @catalog.add_resource copy_resource
>             @catalog.apply
>
>             Dir.entries(@todir).should include("file")
>             Dir.entries(@todir).should include("link")
>         end

Once again, I think this applying of the catalog is unnecessary.  I'd  
accept it, but I'm a bit grumbly about it.

Just to get an idea of what you can do, run this in your test:

   @copy_resource.eval_generate.each { |res| puts res.ref }

That should help you understand what's being generated there and how  
you can use it.

>
> -        it "should default to copy dangling symlinks as links" do
> +        it "should ignore dangling symlinks" do

I think it should actually copy the dangling symlinks here, not ignore  
them, shouldn't it?  At least, if we propagate :links.

>
>             dangling_link = File.join(@basedir, "dangling_link")
>             File.symlink("/some/where/else", dangling_link)
> -            @symlink_resource = Puppet.type(:file).create(
> +            symlink_resource = Puppet.type(:file).create(
>                 :path => @todir,
>                 :source => @basedir,
>                 :ensure => "directory",
>                 :recurse => true
>             )
>


-- 
To be pleased with one's limits is a wretched state.
     -- Johann Wolfgang von Goethe
---------------------------------------------------------------------
Luke Kanies | http://reductivelabs.com | http://madstop.com


--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups 
"Puppet Developers" group.
To post to this group, send email to [email protected]
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at 
http://groups.google.com/group/puppet-dev?hl=en
-~----------~----~----~----~------~----~------~--~---

Reply via email to