Hey Richard,

Thanks for finding where this was caused. We made a slight change to
the patch you submitted to ensure that we always store strings in the
database, avoiding the YAML death-spiral for things other than
Puppet::Node::Environment objects.

Josh

On Apr 26, 2:05 pm, Josh Cooper <[email protected]> wrote:
> Before this change when environment strings were read out of the storeconfigs
> database, they were eventually converted up to Puppet::Node::Environment
> objects. When these objects are returned to the storeconfigs database,
> ActiveRecord dumps them as YAML, which begins the death-spiral of YAML.
>
> This change makes it so the host will always store the environment as a 
> string,
> preventing the Puppet::Node::Environment object from being YAMLized, and 
> stored
> as such in the database.
>
> This change was based on one by Richard Crowley.
>
> Paired-with: Jacob Helwig <[email protected]>
> Reviewed-by: Jesse Wolfe <[email protected]>
> Signed-off-by: Richard Crowley <[email protected]>
> Signed-off-by: Josh Cooper <[email protected]>
> ---
>  lib/puppet/rails/host.rb     |    7 +++++++
>  spec/unit/rails/host_spec.rb |    8 ++++++++
>  2 files changed, 15 insertions(+), 0 deletions(-)
>
> diff --git a/lib/puppet/rails/host.rb b/lib/puppet/rails/host.rb
> index b9dea2a..e536021 100644
> --- a/lib/puppet/rails/host.rb
> +++ b/lib/puppet/rails/host.rb
> @@ -1,3 +1,4 @@
> +require 'puppet/node/environment'
>  require 'puppet/rails'
>  require 'puppet/rails/resource'
>  require 'puppet/rails/fact_name'
> @@ -28,6 +29,12 @@ class Puppet::Rails::Host < ActiveRecord::Base
>      host
>    end
>
> +  # Override the setter for environment to force it to be a string, lest it
> +  # be YAML encoded.  See #4487.
> +  def environment=(value)
> +    super value.to_s
> +  end
> +
>    # returns a hash of fact_names.name => [ fact_values ] for this host.
>    # Note that 'fact_values' is actually a list of the value instances, not
>    # just actual values.
> diff --git a/spec/unit/rails/host_spec.rb b/spec/unit/rails/host_spec.rb
> index 4244f11..b413a16 100755
> --- a/spec/unit/rails/host_spec.rb
> +++ b/spec/unit/rails/host_spec.rb
> @@ -2,6 +2,8 @@
>
>  require File.dirname(__FILE__) + '/../../spec_helper'
>
> +require 'puppet/node/environment'
> +
>  describe "Puppet::Rails::Host", :if => Puppet.features.rails? do
>    def column(name, type)
>      ActiveRecord::ConnectionAdapters::Column.new(name, nil, type, false)
> @@ -43,6 +45,12 @@ describe "Puppet::Rails::Host", :if => 
> Puppet.features.rails? do
>        Puppet::Rails::Host.from_puppet(@node)
>      end
>
> +    it "should stringify the environment" do
> +      host = Puppet::Rails::Host.new
> +      host.environment = Puppet::Node::Environment.new("production")
> +      host.environment.class.should == String
> +    end
> +
>      it "should copy the ipaddress from the Puppet instance" do
>        Puppet::Rails::Host.expects(:find_by_name).with("foo").returns @host
>
> --
> 1.7.4.4

-- 
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