Puppet

can you send the patches to puppet-dev ? –> http://reductivelabs.com/trac/puppet/wiki/Development/DevelopmentLifecycle#submitting-patches thanks!

Sent to the mailing list. Apologies for the confusion. I’ll spend more time reading the development cycle docs.

Al Hoang wrote:

Sent to the mailing list. Apologies for the confusion. I’ll spend more time reading the development cycle docs.

it’s fine to open a ticket as well. oh and pushing the code to some git repository where it can be pulled from, if it have been accepted on the list.

Excluding the duplicated when-clause changes, this branch changes:

• XMLRPC::Service::Interface.new … from one incorrect usage to another; the correct pattern is:

    @interface = XMLRPC::Service::Interface.new("status") {
      add_method("int status()")
    }

The block does not receive an argument, and the present code only works because of a bug in ruby 1.8.x; the code on the branch would work most of the time in any version, but exposes a race condition which is the reason for the block.

• File —> ::File; we need a better fix for this.
• Guards calls to stat with an argument, “stat(false)” whereas they are incorrect and should just be changed to stat with no arguments
• Makes the following change, which I do not understand:

diff --git a/lib/puppet/rails.rb b/lib/puppet/rails.rb
index 87f1bb1..5e51099 100644
--- a/lib/puppet/rails.rb
+++ b/lib/puppet/rails.rb
@@ -55,6 +55,16 @@ module Puppet::Rails
 
             socket          = Puppet[:dbsocket]
             args[:socket]   = socket unless socket.empty?
+          
+            connections     = Puppet[:dbconnections].to_i
+            args[:pool]     = connections if connections > 0 
+        when "oracle_enhanced"
+               args[:database] = Puppet[:dbname] unless Puppet[:dbname].empty?
+               args[:username] = Puppet[:dbuser] unless Puppet[:dbuser].empty?
+               args[:password] = Puppet[:dbpassword] unless Puppet[:dbpassword].empty?
+
+            connections     = Puppet[:dbconnections].to_i
+            args[:pool]     = connections if connections > 0
         else
             raise ArgumentError, "Invalid db adapter %s" % adapter
         end

• Makes the following change which directly related to the title of the ticket:

diff --git a/lib/puppet/util/classgen.rb b/lib/puppet/util/classgen.rb
index 1aa9613..c68fcef 100644
--- a/lib/puppet/util/classgen.rb
+++ b/lib/puppet/util/classgen.rb
@@ -128,11 +128,23 @@ module Puppet::Util::ClassGen
         return klass
     end
 
+    # const_defined? in Ruby 1.9 behaves differently in terms
+    # of which class hierarchy it polls for nested namespaces
+    #
+    # See http://redmine.ruby-lang.org/issues/show/1915
+    def is_constant_defined?(const)
+      if ::RUBY_VERSION =~ /1.9/
+        const_defined?(const, false)
+      else
+        const_defined?(const)
+      end
+    end
+
     # Handle the setting and/or removing of the associated constant.
     def handleclassconst(klass, name, options)
         const = genconst_string(name, options)
 
-        if const_defined? const
+        if is_constant_defined?(const)
             if options[:overwrite]
                 Puppet.info "Redefining %s in %s" % [name, self]
                 remove_const(const)

Hi there,

Since I originated this ticket / patch request I think I should chime in just to clarify some things.

Markus Roberts wrote:

I’m marking this “Code Insufficient”

I think this is reasonable. I started work on getting Puppet to run on Ruby 1.9 however I’ve not had a lot of time to look into this recently but have been wanting to pick it up again and complete it.

1. Contains fragments of an older version of #2781
2. Does not merge cleanly on master

This makes sense and I was hoping to keep up with upstream in my branch on github however it has fallen way behind.

1. Duplicates some of the changes that are being made in the code smell process

I’m not sure I understand what this means. If you had time to explain this, that would be very helpful. Otherwise, I’ll take your word for it.

1. Fixes things in ways that don’t make sense (e.g. guarding @resource.stat(false) with a version check when none of the stat methods took a boolean parameter even in 1.8)

Thanks for pointing this out. I’d prefer a cleaner way to get it working. I believe you suggested a cleaner methodology awhile back on the mailing list but I’ve not had time to go back and review your suggestions again. The changesets I have on github reflected my really strong desire (at the time) to have something running over a clean implementation so please take that github branch with a grain of salt.

My intent is to manually go through the changes after the dust settles and come up with a patch that addresses the things that need to be fixed with a new branch, perhaps based on this one.

This sounds reasonable to me. If you know of a way to perhaps split what needs to be done such that I can help that would be great.