(continued)
diff --git a/lib/puppet/type/file/checksum.rb
> b/lib/puppet/type/file/checksum.rb
> index 5af943e..5b99bef 100755
> --- a/lib/puppet/type/file/checksum.rb
> +++ b/lib/puppet/type/file/checksum.rb
> @@ -23,4 +23,11 @@ Puppet::Type.type(:file).newparam(:checksum) do
> method = type.to_s + "_file"
> "{#{type}}" + send(method, path).to_s
> end
> +
> + def sum_stream(&block)
> + type = value || :md5 # same comment as above
> + method = type.to_s + "_stream"
> + checksum = send(method, &block)
> + "{#{type}}#{checksum}"
> + end
> end
> diff --git a/lib/puppet/type/file/content.rb
> b/lib/puppet/type/file/content.rb
> index b6c671b..3789900 100755
> --- a/lib/puppet/type/file/content.rb
> +++ b/lib/puppet/type/file/content.rb
> @@ -1,9 +1,14 @@
> +require 'net/http'
> +require 'uri'
> +
> require 'puppet/util/checksums'
> +require 'puppet/network/http/api/v1'
>
> module Puppet
> Puppet::Type.type(:file).newproperty(:content) do
> include Puppet::Util::Diff
> include Puppet::Util::Checksums
> + include Puppet::Network::HTTP::API::V1
>
> desc "Specify the contents of a file as a string. Newlines, tabs,
> and
> spaces can be specified using the escaped syntax (e.g., \\n for
> a
> @@ -68,21 +73,22 @@ module Puppet
> end
> end
>
> - # If content was specified, return that; else try to return the
> source content;
> + # If content was specified, return that;
> # else, return nil.
> def actual_content
> if defined?(@actual_content) and @actual_content
> return @actual_content
> end
>
> - if s = resource.parameter(:source)
> - return s.content
> - end
> - fail "Could not find actual content from checksum"
> + return nil
> + end
> +
>
:-) So that would be:
def actual_content
@actual_content
end
or even
attr_reader :actual_content
> + def length
> + (actual_content and actual_content.length) || 0
> end
>
> def content
> - self.should || (s = resource.parameter(:source) and s.content)
> + self.should
> end
>
> # Override this method to provide diffs if asked for.
> @@ -140,9 +146,62 @@ module Puppet
> # We're safe not testing for the 'source' if there's no
> 'should'
> # because we wouldn't have gotten this far if there weren't at
> least
> # one valid value somewhere.
> - @resource.write(actual_content, :content)
> + @resource.write(:content)
>
> return return_event
> end
> +
> + def write(file)
> + return write_content(file) if actual_content
> + return write_source(file) if resource.parameter(:source)
> + self.fail "Writing content that wasn't provided"
> + end
> +
> + def write_content(file)
> + file.print actual_content
> + return resource.parameter(:checksum).sum(actual_content)
> + end
> +
> + def write_source(file)
> + if source = resource.parameter(:source)
> + return write_local_source(file, source) if source.local?
> + return write_remote_source(file, source)
> + end
> + end
> +
> + def write_local_source(file, source)
> + resource.parameter(:checksum).sum_stream do |sum|
> + File.open(source.full_path, "r") do |src|
> + puts "local open"
>
I assume the puts should be removed?
> + while chunk = src.read(8192)
> + sum << chunk
> + file.print chunk
> + end
> + end
> + end
> + end
> +
> + def write_remote_source(file, source)
> + request = Puppet::Indirector::Request.new(:file_content,
> :find, source.full_path)
> + checksum = nil
> + Puppet::Network::HttpPool.http_instance(source.server,
> source.port).request_get(indirection2uri(request), {"Accept" => "raw"}) do
> |response|
> + case response.code
> + when "404"
> + return nil
> + when /^2/
> + checksum = resource.parameter(:checksum).sum_stream do
> |sum|
> + response.read_body do |chunk|
> + sum << chunk
> + file.print chunk
> + end
>
+ end
> + else
> + # Raise the http error if we didn't get a 'success' of
> some kind.
> + message = "Error %s on SERVER: %s" % [response.code,
> (response.body||'').empty? ? response.message : response.body]
> + raise Net::HTTPError.new(message, response)
> + end
> + end
> + return checksum
> + end
> end
> end
>
Hmmm. More thinking than recommending, but what about something like:
def write_source(file)
resource.parameter(:checksum).sum_stream { |sum|
each_chunk_from(file,resource.parameter(:source) { |chunk|
sum << chunk
file.print chunk
}
}
end
def each_chunk_from(file,source)
if source.nil?
nil
elsif source.local?
File.open(source.full_path, "r") do |src|
while chunk = src.read(8192)
yield chunk
end
end
else
request = Puppet::Indirector::Request.new(:file_content,
:find, source.full_path)
connection =
Puppet::Network::HttpPool.http_instance(source.server, source.port).
connection.request_get(indirection2uri(request), {"Accept"
=> "raw"}) do |response|
case response.code
when "404"; nil
when /^2/; response.read_body { |chunk| yield chunk }
else
# Raise the http error if we didn't get a 'success'
of some kind.
message = "Error %s on SERVER: %s" % [response.code,
(response.body||'').empty? ? response.message : response.body]
raise Net::HTTPError.new(message, response)
end
}
end
end
In other words, rather than pushing the checksuming and writing into the
walk the stream code, keep it at the write_source level and push the
local/remote choice down instead? If this direction is good, it looks like
we could go even further and unify write_source and write_content, by adding
the content logic at the head of each_chunk_from.
> diff --git a/lib/puppet/type/file/ensure.rb
> b/lib/puppet/type/file/ensure.rb
> index 5f0beee..b8a85f6 100755
> --- a/lib/puppet/type/file/ensure.rb
> +++ b/lib/puppet/type/file/ensure.rb
> @@ -45,7 +45,7 @@ module Puppet
> if property = @resource.property(:content)
> property.sync
> else
> - @resource.write("", :ensure)
> + @resource.write(:ensure)
> mode = @resource.should(:mode)
> end
> return :file_created
> diff --git a/lib/puppet/type/file/source.rb
> b/lib/puppet/type/file/source.rb
> index 838dabb..5b86e82 100755
> --- a/lib/puppet/type/file/source.rb
> +++ b/lib/puppet/type/file/source.rb
> @@ -96,16 +96,6 @@ module Puppet
> metadata && metadata.checksum
> end
>
> - # Look up (if necessary) and return remote content.
> - cached_attr(:content) do
> - raise Puppet::DevError, "No source for content was stored with
> the metadata" unless metadata.source
> -
> - unless tmp =
> Puppet::FileServing::Content.find(metadata.source)
> - fail "Could not find any content at %s" % metadata.source
> - end
> - tmp.content
> - end
> -
> # Copy the values from the source to the resource. Yay.
> def copy_source_values
> devfail "Somehow got asked to copy source values without any
> metadata" unless metadata
> @@ -171,5 +161,32 @@ module Puppet
> resource[:check] = checks
> resource[:checksum] = :md5 unless resource.property(:checksum)
> end
> +
> + def local?
> + if found? and uri
> + return uri.scheme == "file" || uri.scheme == nil
> + end
> + false
> + end
>
How about this (reading "||" as "defaults to"):
def local?
found? and uri and (uri.scheme || "file") == "file"
end
> + def full_path
> + if found? and uri
> + return URI.unescape(uri.path)
> + end
> + end
> +
> + def server
> + (uri and uri.host) or Puppet.settings[:server]
> + end
> +
> + def port
> + (uri and uri.port) or Puppet.settings[:masterport]
> + end
> +
> + private
> +
> + def uri
> + @uri ||= URI.parse(URI.escape(metadata.source))
> + end
> end
> end
> diff --git a/lib/puppet/util/checksums.rb b/lib/puppet/util/checksums.rb
> index b0dcf80..a7aeaea 100644
> --- a/lib/puppet/util/checksums.rb
> +++ b/lib/puppet/util/checksums.rb
> @@ -39,11 +39,27 @@ module Puppet::Util::Checksums
> md5_file(filename, true)
> end
>
> + def md5_stream(&block)
> + require 'digest/md5'
> + digest = Digest::MD5.new()
> + yield digest
> + return digest.hexdigest
> + end
> +
> + alias :md5lite_stream :md5_stream
> +
> # Return the :mtime timestamp of a file.
> def mtime_file(filename)
> File.stat(filename).send(:mtime)
> end
>
> + # by definition this doesn't exist
> + def mtime_stream
> + nil
> + end
> +
> + alias :ctime_stream :mtime_stream
> +
> # Calculate a checksum using Digest::SHA1.
> def sha1(content)
> require 'digest/sha1'
> @@ -68,6 +84,15 @@ module Puppet::Util::Checksums
> sha1_file(filename, true)
> end
>
> + def sha1_stream
> + require 'digest/sha1'
> + digest = Digest::SHA1.new()
> + yield digest
> + return digest.hexdigest
> + end
> +
> + alias :sha1lite_stream :sha1_stream
> +
> # Return the :ctime of a file.
> def ctime_file(filename)
> File.stat(filename).send(:ctime)
>
Final thought: I think this has the most thorough test set of any commit
I've seen this year.
-- Makus
--
You received this message because you are subscribed to the Google Groups
"Puppet Developers" group.
To post to this group, send email to puppet-...@googlegroups.com.
To unsubscribe from this group, send email to
puppet-dev+unsubscr...@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/puppet-dev?hl=en.
