[Pkg-puppet-devel] [SCM] Puppet packaging for Debian branch, experimental, updated. debian/2.6.8-1-844-g7ec39d5

[Daniel Pittman]

The following commit has been merged in the experimental branch:
commit 5938452dccc8c925bc6275a62ae96f50916cc239
Author: Daniel Pittman <daniel at puppetlabs.com>
Date:   Mon Apr 18 15:23:03 2011 -0700

    (#7013) Strip out old face-wide rendering defaults.
    
    Now we want to support action-based rendering, it is super-hard to define the
    semantics around defaulting where things are unspecified: the execution
    context (CLI, HTTP, etc) vs the face, vs the action all have different
    semantics.
    
    Without solving the problem of how we express all that context and those
    semantics down in the action, especially one written by a third party, this
    just becomes a box of counter-intuitive and annoying semantics and edge-cases.
    
    Reviewed-By: Max Martin <max at puppetlabs.com>

diff --git a/lib/puppet/application/face_base.rb b/lib/puppet/application/face_base.rb
index 947204d..456f0c6 100644
--- a/lib/puppet/application/face_base.rb
+++ b/lib/puppet/application/face_base.rb
@@ -15,8 +15,8 @@ class Puppet::Application::FaceBase < Puppet::Application
     Puppet::Util::Log.level = :info
   end
 
-  option("--format FORMAT") do |arg|
-    @format = arg.to_sym
+  option("--render-as FORMAT") do |arg|
+    @render_as = arg.to_sym
   end
 
   option("--mode RUNMODE", "-r") do |arg|
@@ -26,7 +26,7 @@ class Puppet::Application::FaceBase < Puppet::Application
   end
 
 
-  attr_accessor :face, :action, :type, :arguments, :format
+  attr_accessor :face, :action, :type, :arguments, :render_as
   attr_writer :exit_code
 
   # This allows you to set the exit code if you don't want to just exit
@@ -37,8 +37,8 @@ class Puppet::Application::FaceBase < Puppet::Application
 
   # Override this if you need custom rendering.
   def render(result)
-    if format then
-      render_method = Puppet::Network::FormatHandler.format(format).render_method
+    if render_as then
+      render_method = Puppet::Network::FormatHandler.format(render_as).render_method
       if render_method == "to_pson"
         jj result
       else
@@ -85,9 +85,8 @@ class Puppet::Application::FaceBase < Puppet::Application
 
     # REVISIT: These should be configurable versions, through a global
     # '--version' option, but we don't implement that yet... --daniel 2011-03-29
-    @type   = self.class.name.to_s.sub(/.+:/, '').downcase.to_sym
-    @face   = Puppet::Face[@type, :current]
-    @format = @face.default_format
+    @type      = self.class.name.to_s.sub(/.+:/, '').downcase.to_sym
+    @face      = Puppet::Face[@type, :current]
 
     # Now, walk the command line and identify the action.  We skip over
     # arguments based on introspecting the action and all, and find the first
diff --git a/lib/puppet/face/facts.rb b/lib/puppet/face/facts.rb
index 8668b25..04eab93 100644
--- a/lib/puppet/face/facts.rb
+++ b/lib/puppet/face/facts.rb
@@ -2,10 +2,10 @@ require 'puppet/face/indirector'
 require 'puppet/node/facts'
 
 Puppet::Face::Indirector.define(:facts, '0.0.1') do
-  set_default_format :yaml
-
   # Upload our facts to the server
   action(:upload) do
+    render_as :yaml
+
     when_invoked do |options|
       Puppet::Node::Facts.indirection.terminus_class = :facter
       facts = Puppet::Node::Facts.indirection.find(Puppet[:certname])
diff --git a/lib/puppet/face/node.rb b/lib/puppet/face/node.rb
index fd1a548..12336df 100644
--- a/lib/puppet/face/node.rb
+++ b/lib/puppet/face/node.rb
@@ -1,5 +1,3 @@
 require 'puppet/face/indirector'
-
 Puppet::Face::Indirector.define(:node, '0.0.1') do
-  set_default_format :yaml
 end
diff --git a/lib/puppet/interface.rb b/lib/puppet/interface.rb
index 51ae0cd..398d854 100644
--- a/lib/puppet/interface.rb
+++ b/lib/puppet/interface.rb
@@ -62,10 +62,8 @@ class Puppet::Interface
     end
   end
 
-  attr_accessor :default_format
-
   def set_default_format(format)
-    self.default_format = format.to_sym
+    Puppet.warning("set_default_format is deprecated (and ineffective); use render_as on your actions instead.")
   end
 
   attr_accessor :summary
@@ -83,7 +81,6 @@ class Puppet::Interface
 
     @name = Puppet::Interface::FaceCollection.underscorize(name)
     @version = version
-    @default_format = :pson
 
     instance_eval(&block) if block_given?
   end
diff --git a/lib/puppet/interface/action.rb b/lib/puppet/interface/action.rb
index 53c40bc..8bacc21 100644
--- a/lib/puppet/interface/action.rb
+++ b/lib/puppet/interface/action.rb
@@ -9,9 +9,9 @@ class Puppet::Interface::Action
     @name    = name.to_sym
     attrs.each do |k, v| send("#{k}=", v) end
 
-    @options = {}
+    @options        = {}
     @when_rendering = {}
-    @render_as = :for_humans
+    @render_as      = :for_humans
   end
 
   # This is not nice, but it is the easiest way to make us behave like the
diff --git a/spec/unit/application/face_base_spec.rb b/spec/unit/application/face_base_spec.rb
index 0d39aa1..6a46472 100755
--- a/spec/unit/application/face_base_spec.rb
+++ b/spec/unit/application/face_base_spec.rb
@@ -59,10 +59,6 @@ describe Puppet::Application::FaceBase do
           app.face.name.should == :basetest
         end
 
-        it "should set the format based on the face default" do
-          app.format.should == :pson
-        end
-
         it "should find the action" do
           app.action.should be
           app.action.name.should == :foo
@@ -192,7 +188,6 @@ describe Puppet::Application::FaceBase do
 
       app.face      = Puppet::Face[:basetest, '0.0.1']
       app.action    = app.face.get_action(:foo)
-      app.format    = :pson
       app.arguments = ["myname", "myarg"]
     end
 
diff --git a/spec/unit/face/facts_spec.rb b/spec/unit/face/facts_spec.rb
index e6411f8..6ab6ad5 100755
--- a/spec/unit/face/facts_spec.rb
+++ b/spec/unit/face/facts_spec.rb
@@ -2,14 +2,10 @@
 require 'spec_helper'
 
 describe Puppet::Face[:facts, '0.0.1'] do
-  it "should define an 'upload' fact" do
+  it "should define an 'upload' action" do
     subject.should be_action(:upload)
   end
 
-  it "should set its default format to :yaml" do
-    subject.default_format.should == :yaml
-  end
-
   describe "when uploading" do
     it "should set the terminus_class to :facter"
 
diff --git a/spec/unit/face/node_spec.rb b/spec/unit/face/node_spec.rb
index 90d258d..d19312c 100755
--- a/spec/unit/face/node_spec.rb
+++ b/spec/unit/face/node_spec.rb
@@ -2,7 +2,5 @@
 require 'spec_helper'
 
 describe Puppet::Face[:node, '0.0.1'] do
-  it "should set its default format to :yaml" do
-    subject.default_format.should == :yaml
-  end
+  it "REVISIT: really should have some tests"
 end
diff --git a/spec/unit/interface_spec.rb b/spec/unit/interface_spec.rb
index e52b45d..799b8c4 100755
--- a/spec/unit/interface_spec.rb
+++ b/spec/unit/interface_spec.rb
@@ -99,16 +99,6 @@ describe Puppet::Interface do
     subject.new(:me, '0.0.1').to_s.should =~ /\bme\b/
   end
 
-  it "should allow overriding of the default format" do
-    face = subject.new(:me, '0.0.1')
-    face.set_default_format :foo
-    face.default_format.should == :foo
-  end
-
-  it "should default to :pson for its format" do
-    subject.new(:me, '0.0.1').default_format.should == :pson
-  end
-
   # Why?
   it "should create a class-level autoloader" do
     subject.autoloader.should be_instance_of(Puppet::Util::Autoload)

-- 
Puppet packaging for Debian