[Puppet-dev] [PATCH/puppet 1/1] Fixes #3582 - Adds dbport configuration option for specifying database port

[James Turnbull]

Signed-off-by: James Turnbull <ja...@lovedthanlost.net>
---
ext/puppetstoredconfigclean.rb | 1 +
lib/puppet/defaults.rb | 2 ++
lib/puppet/rails.rb | 1 +
spec/unit/rails.rb | 19 ++++++++++++++-----
4 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/ext/puppetstoredconfigclean.rb b/ext/puppetstoredconfigclean.rb
index 978e083..1b4aabd 100644
--- a/ext/puppetstoredconfigclean.rb
+++ b/ext/puppetstoredconfigclean.rb
@@ -64,6 +64,7 @@ case adapter
args[:username] = pm_conf[:dbuser] unless pm_conf[:dbuser].to_s.empty?
args[:password] = pm_conf[:dbpassword] unless pm_conf[:dbpassword].to_s.empty?
args[:database] = pm_conf[:dbname] unless pm_conf[:dbname].to_s.empty?
+ args[:port] = pm_conf[:dbport] unless pm_conf[:dbport].to_s.empty?
socket = pm_conf[:dbsocket]
args[:socket] = socket unless socket.to_s.empty?
connections = pm_conf[:dbconnections].to_i
diff --git a/lib/puppet/defaults.rb b/lib/puppet/defaults.rb
index 2f397f4..27d257c 100644
--- a/lib/puppet/defaults.rb
+++ b/lib/puppet/defaults.rb
@@ -674,6 +674,8 @@ module Puppet
:dbname => [ "puppet", "The name of the database to use." ],
:dbserver => [ "localhost", "The database server for Client caching. Only
used when networked databases are used."],
+ :dbport => [ "", "The database password for Client caching. Only
+ used when networked databases are used."],
:dbuser => [ "puppet", "The database user for Client caching. Only
used when networked databases are used."],
:dbpassword => [ "puppet", "The database password for Client caching. Only
diff --git a/lib/puppet/rails.rb b/lib/puppet/rails.rb
index 1e02408..c3bf383 100644
--- a/lib/puppet/rails.rb
+++ b/lib/puppet/rails.rb
@@ -51,6 +51,7 @@ module Puppet::Rails
args[:database] = Puppet[:dblocation]
when "mysql", "postgresql"
args[:host] = Puppet[:dbserver] unless Puppet[:dbserver].empty?
+ args[:port] = Puppet[:dbport] unless Puppet[:dbport].empty?
args[:username] = Puppet[:dbuser] unless Puppet[:dbuser].empty?
args[:password] = Puppet[:dbpassword] unless Puppet[:dbpassword].empty?
args[:database] = Puppet[:dbname]
diff --git a/spec/unit/rails.rb b/spec/unit/rails.rb
index 944bf45..b16e6be 100755
--- a/spec/unit/rails.rb
+++ b/spec/unit/rails.rb
@@ -90,10 +90,11 @@ end
describe Puppet::Rails, "when initializing a mysql connection" do
confine "Cannot test without ActiveRecord" => Puppet.features.rails?

- it "should provide the adapter, log_level, and host, username, password, database, and reconnect arguments" do
+ it "should provide the adapter, log_level, and host, port, username, password, database, and reconnect arguments" do
Puppet.settings.stubs(:value).with(:dbadapter).returns("mysql")
Puppet.settings.stubs(:value).with(:rails_loglevel).returns("testlevel")
Puppet.settings.stubs(:value).with(:dbserver).returns("testserver")
+ Puppet.settings.stubs(:value).with(:dbport).returns("")
Puppet.settings.stubs(:value).with(:dbuser).returns("testuser")
Puppet.settings.stubs(:value).with(:dbpassword).returns("testpassword")
Puppet.settings.stubs(:value).with(:dbname).returns("testname")
@@ -112,10 +113,11 @@ describe Puppet::Rails, "when initializing a mysql connection" do
}
end

- it "should provide the adapter, log_level, and host, username, password, database, socket, connections, and reconnect arguments" do
+ it "should provide the adapter, log_level, and host, port, username, password, database, socket, connections, and reconnect arguments" do
Puppet.settings.stubs(:value).with(:dbadapter).returns("mysql")
Puppet.settings.stubs(:value).with(:rails_loglevel).returns("testlevel")
Puppet.settings.stubs(:value).with(:dbserver).returns("testserver")
+ Puppet.settings.stubs(:value).with(:dbport).returns("9999")
Puppet.settings.stubs(:value).with(:dbuser).returns("testuser")
Puppet.settings.stubs(:value).with(:dbpassword).returns("testpassword")
Puppet.settings.stubs(:value).with(:dbname).returns("testname")
@@ -126,6 +128,7 @@ describe Puppet::Rails, "when initializing a mysql connection" do
:adapter => "mysql",
:log_level => "testlevel",
:host => "testserver",
+ :port => "9999",
:username => "testuser",
:password => "testpassword",
:database => "testname",
@@ -135,10 +138,11 @@ describe Puppet::Rails, "when initializing a mysql connection" do
}
end

- it "should provide the adapter, log_level, and host, username, password, database, socket, and connections arguments" do
+ it "should provide the adapter, log_level, and host, port, username, password, database, socket, and connections arguments" do
Puppet.settings.stubs(:value).with(:dbadapter).returns("mysql")
Puppet.settings.stubs(:value).with(:rails_loglevel).returns("testlevel")
Puppet.settings.stubs(:value).with(:dbserver).returns("testserver")
+ Puppet.settings.stubs(:value).with(:dbport).returns("9999")
Puppet.settings.stubs(:value).with(:dbuser).returns("testuser")
Puppet.settings.stubs(:value).with(:dbpassword).returns("testpassword")
Puppet.settings.stubs(:value).with(:dbname).returns("testname")
@@ -149,6 +153,7 @@ describe Puppet::Rails, "when initializing a mysql connection" do
:adapter => "mysql",
:log_level => "testlevel",
:host => "testserver",
+ :port => "9999",
:username => "testuser",
:password => "testpassword",
:database => "testname",
@@ -162,10 +167,11 @@ end
describe Puppet::Rails, "when initializing a postgresql connection" do
confine "Cannot test without ActiveRecord" => Puppet.features.rails?

- it "should provide the adapter, log_level, and host, username, password, connections, and database arguments" do
+ it "should provide the adapter, log_level, and host, port, username, password, connections, and database arguments" do
Puppet.settings.stubs(:value).with(:dbadapter).returns("postgresql")
Puppet.settings.stubs(:value).with(:rails_loglevel).returns("testlevel")
Puppet.settings.stubs(:value).with(:dbserver).returns("testserver")
+ Puppet.settings.stubs(:value).with(:dbport).returns("9999")
Puppet.settings.stubs(:value).with(:dbuser).returns("testuser")
Puppet.settings.stubs(:value).with(:dbpassword).returns("testpassword")
Puppet.settings.stubs(:value).with(:dbname).returns("testname")
@@ -176,6 +182,7 @@ describe Puppet::Rails, "when initializing a postgresql connection" do
:adapter => "postgresql",
:log_level => "testlevel",
:host => "testserver",
+ :port => "9999",
:username => "testuser",
:password => "testpassword",
:database => "testname",
@@ -184,10 +191,11 @@ describe Puppet::Rails, "when initializing a postgresql connection" do
}
end

- it "should provide the adapter, log_level, and host, username, password, database, connections, and socket arguments" do
+ it "should provide the adapter, log_level, and host, port, username, password, database, connections, and socket arguments" do
Puppet.settings.stubs(:value).with(:dbadapter).returns("postgresql")
Puppet.settings.stubs(:value).with(:rails_loglevel).returns("testlevel")
Puppet.settings.stubs(:value).with(:dbserver).returns("testserver")
+ Puppet.settings.stubs(:value).with(:dbport).returns("9999")
Puppet.settings.stubs(:value).with(:dbuser).returns("testuser")
Puppet.settings.stubs(:value).with(:dbpassword).returns("testpassword")
Puppet.settings.stubs(:value).with(:dbname).returns("testname")
@@ -198,6 +206,7 @@ describe Puppet::Rails, "when initializing a postgresql connection" do
:adapter => "postgresql",
:log_level => "testlevel",
:host => "testserver",
+ :port => "9999",
:username => "testuser",
:password => "testpassword",
:database => "testname",
--
1.6.6.1

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

[Luke Kanies]

+1, with one question:

Is this really "for Client caching"? It's for catalog caching, right?

Those docs might need to be fixed.

> .settings.stubs(:value).with(:dbpassword).returns("testpassword")
>
> Puppet.settings.stubs(:value).with(:dbname).returns("testname")
> @@ -149,6 +153,7 @@ describe Puppet::Rails, "when initializing a
> mysql connection" do
> :adapter => "mysql",
> :log_level => "testlevel",
> :host => "testserver",
> + :port => "9999",
> :username => "testuser",
> :password => "testpassword",
> :database => "testname",
> @@ -162,10 +167,11 @@ end
> describe Puppet::Rails, "when initializing a postgresql connection" do
> confine "Cannot test without ActiveRecord" =>
> Puppet.features.rails?
>
> - it "should provide the adapter, log_level, and host, username,
> password, connections, and database arguments" do
> + it "should provide the adapter, log_level, and host, port,
> username, password, connections, and database arguments" do
>
> Puppet.settings.stubs(:value).with(:dbadapter).returns("postgresql")
>
> Puppet

> .settings.stubs(:value).with(:rails_loglevel).returns("testlevel")
>
> Puppet.settings.stubs(:value).with(:dbserver).returns("testserver")
> + Puppet.settings.stubs(:value).with(:dbport).returns("9999")
>
> Puppet.settings.stubs(:value).with(:dbuser).returns("testuser")
>
> Puppet

> .settings.stubs(:value).with(:dbpassword).returns("testpassword")
>
> Puppet.settings.stubs(:value).with(:dbname).returns("testname")
> @@ -176,6 +182,7 @@ describe Puppet::Rails, "when initializing a
> postgresql connection" do
> :adapter => "postgresql",
> :log_level => "testlevel",
> :host => "testserver",
> + :port => "9999",
> :username => "testuser",
> :password => "testpassword",
> :database => "testname",
> @@ -184,10 +191,11 @@ describe Puppet::Rails, "when initializing a
> postgresql connection" do
> }
> end
>
> - it "should provide the adapter, log_level, and host, username,
> password, database, connections, and socket arguments" do
> + it "should provide the adapter, log_level, and host, port,
> username, password, database, connections, and socket arguments" do
>
> Puppet.settings.stubs(:value).with(:dbadapter).returns("postgresql")
>
> Puppet

> .settings.stubs(:value).with(:rails_loglevel).returns("testlevel")
>
> Puppet.settings.stubs(:value).with(:dbserver).returns("testserver")
> + Puppet.settings.stubs(:value).with(:dbport).returns("9999")
>
> Puppet.settings.stubs(:value).with(:dbuser).returns("testuser")
>
> Puppet

> .settings.stubs(:value).with(:dbpassword).returns("testpassword")
>
> Puppet.settings.stubs(:value).with(:dbname).returns("testname")
> @@ -198,6 +206,7 @@ describe Puppet::Rails, "when initializing a
> postgresql connection" do
> :adapter => "postgresql",
> :log_level => "testlevel",
> :host => "testserver",
> + :port => "9999",
> :username => "testuser",
> :password => "testpassword",
> :database => "testname",
> --
> 1.6.6.1
>
> --
> You received this message because you are subscribed to the Google
> Groups "Puppet Developers" group.
> To post to this group, send email to puppe...@googlegroups.com.
> To unsubscribe from this group, send email to puppet-dev+...@googlegroups.com
> .
> For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en
> .
>

--
Men never do evil so completely and cheerfully as when they do it from a
religious conviction. --Blaise Pascal
---------------------------------------------------------------------
Luke Kanies -|- http://puppetlabs.com -|- +1(615)594-8199

[Krzysztof Wilczynski]

James,

I might be completely wrong, but ... I believe that the following may
throw an exception as "empty" is not a known method for Fixnum since
during the parsing stage in settings.rb we munge arguments and cast
them according to what type they look like (upon matching with regular
expression), and it will be Integer in this case:

+ args[:port] = Puppet[:dbport] unless
Puppet[:dbport].empty?

This probably should say:

+ args[:port] = Puppet[:dbport] unless

Puppet[:dbport].zero?

Said that, I reckon this has to be changed as well:

+ args[:port] = pm_conf[:dbport] unless
pm_conf[:dbport].to_s.empty?

To:

+ args[:port] = pm_conf[:dbport].to_i unless
pm_conf[:dbport].to_i.zero?

[...]

Perhaps rake tests also need some love in that regards. Anyway, does
this makes any sense? :-)

KW

[Krzysztof Wilczynski]

James,

Take a look on this delta below and let me know what you think.

diff --git a/ext/puppetstoredconfigclean.rb b/ext/
puppetstoredconfigclean.rb

index 978e083..65ed8df 100644
--- a/ext/puppetstoredconfigclean.rb
+++ b/ext/puppetstoredconfigclean.rb
@@ -61,6 +61,7 @@ case adapter
args[:dbfile] = pm_conf[:dblocation]
when "mysql", "postgresql"
args[:host] = pm_conf[:dbserver] unless
pm_conf[:dbserver].to_s.empty?

+ args[:port] = pm_conf[:dbport].to_i unless
pm_conf[:dbport].to_i.zero?

args[:username] = pm_conf[:dbuser] unless
pm_conf[:dbuser].to_s.empty?
args[:password] = pm_conf[:dbpassword] unless
pm_conf[:dbpassword].to_s.empty?
args[:database] = pm_conf[:dbname] unless
pm_conf[:dbname].to_s.empty?

diff --git a/lib/puppet/defaults.rb b/lib/puppet/defaults.rb
index 2f397f4..c69b1cb 100644

--- a/lib/puppet/defaults.rb
+++ b/lib/puppet/defaults.rb
@@ -674,6 +674,8 @@ module Puppet
:dbname => [ "puppet", "The name of the database to use." ],
:dbserver => [ "localhost", "The database server for Client
caching. Only
used when networked databases are used."],

+ :dbport => [ '', "The port on which the database server
listens for connections.
+ Will fall-back to default setting if the value is an
empty string." ],

:dbuser => [ "puppet", "The database user for Client caching.
Only
used when networked databases are used."],
:dbpassword => [ "puppet", "The database password for Client
caching. Only
diff --git a/lib/puppet/rails.rb b/lib/puppet/rails.rb

index 1e02408..7b743de 100644

--- a/lib/puppet/rails.rb
+++ b/lib/puppet/rails.rb
@@ -51,6 +51,7 @@ module Puppet::Rails
args[:database] = Puppet[:dblocation]
when "mysql", "postgresql"
args[:host] = Puppet[:dbserver] unless
Puppet[:dbserver].empty?

+ args[:port] = Puppet[:dbport] unless
Puppet[:dbport].zero?

args[:username] = Puppet[:dbuser] unless
Puppet[:dbuser].empty?
args[:password] = Puppet[:dbpassword] unless
Puppet[:dbpassword].empty?
args[:database] = Puppet[:dbname]
diff --git a/spec/unit/rails.rb b/spec/unit/rails.rb

index 944bf45..91720a1 100755

--- a/spec/unit/rails.rb
+++ b/spec/unit/rails.rb
@@ -90,10 +90,11 @@ end
describe Puppet::Rails, "when initializing a mysql connection" do
confine "Cannot test without ActiveRecord" =>
Puppet.features.rails?

- it "should provide the adapter, log_level, and host, username,
password, database, and reconnect arguments" do
+ it "should provide the adapter, log_level, and host, port,
username, password, database, and reconnect arguments" do

Puppet.settings.stubs(:value).with(:dbadapter).returns("mysql")

Puppet.settings.stubs(:value).with(:rails_loglevel).returns("testlevel")

Puppet.settings.stubs(:value).with(:dbserver).returns("testserver")
+ Puppet.settings.stubs(:value).with(:dbport).returns("")

Puppet.settings.stubs(:value).with(:dbuser).returns("testuser")

Puppet.settings.stubs(:value).with(:dbpassword).returns("testpassword")

Puppet.settings.stubs(:value).with(:dbname).returns("testname")
@@ -112,10 +113,11 @@ describe Puppet::Rails, "when initializing a
mysql connection" do
}
end

- it "should provide the adapter, log_level, and host, username,
password, database, socket, connections, and reconnect arguments" do
+ it "should provide the adapter, log_level, and host, port,
username, password, database, socket, connections, and reconnect
arguments" do

Puppet.settings.stubs(:value).with(:dbadapter).returns("mysql")

Puppet.settings.stubs(:value).with(:rails_loglevel).returns("testlevel")

Puppet.settings.stubs(:value).with(:dbserver).returns("testserver")

+ Puppet.settings.stubs(:value).with(:dbport).returns(8192)

Puppet.settings.stubs(:value).with(:dbuser).returns("testuser")

Puppet.settings.stubs(:value).with(:dbpassword).returns("testpassword")

Puppet.settings.stubs(:value).with(:dbname).returns("testname")
@@ -126,6 +128,7 @@ describe Puppet::Rails, "when initializing a mysql
connection" do
:adapter => "mysql",
:log_level => "testlevel",
:host => "testserver",

+ :port => 8192,

:username => "testuser",
:password => "testpassword",
:database => "testname",
@@ -135,10 +138,11 @@ describe Puppet::Rails, "when initializing a
mysql connection" do
}
end

- it "should provide the adapter, log_level, and host, username,
password, database, socket, and connections arguments" do
+ it "should provide the adapter, log_level, and host, port,
username, password, database, socket, and connections arguments" do

Puppet.settings.stubs(:value).with(:dbadapter).returns("mysql")

Puppet.settings.stubs(:value).with(:rails_loglevel).returns("testlevel")

Puppet.settings.stubs(:value).with(:dbserver).returns("testserver")

+ Puppet.settings.stubs(:value).with(:dbport).returns(8192)

Puppet.settings.stubs(:value).with(:dbuser).returns("testuser")

Puppet.settings.stubs(:value).with(:dbpassword).returns("testpassword")

Puppet.settings.stubs(:value).with(:dbname).returns("testname")
@@ -149,6 +153,7 @@ describe Puppet::Rails, "when initializing a mysql
connection" do
:adapter => "mysql",
:log_level => "testlevel",
:host => "testserver",

+ :port => 8192,

:username => "testuser",
:password => "testpassword",
:database => "testname",
@@ -162,10 +167,11 @@ end
describe Puppet::Rails, "when initializing a postgresql connection"
do
confine "Cannot test without ActiveRecord" =>
Puppet.features.rails?

- it "should provide the adapter, log_level, and host, username,
password, connections, and database arguments" do
+ it "should provide the adapter, log_level, and host, port,
username, password, connections, and database arguments" do

Puppet.settings.stubs(:value).with(:dbadapter).returns("postgresql")

Puppet.settings.stubs(:value).with(:rails_loglevel).returns("testlevel")

Puppet.settings.stubs(:value).with(:dbserver).returns("testserver")

+ Puppet.settings.stubs(:value).with(:dbport).returns(8192)

Puppet.settings.stubs(:value).with(:dbuser).returns("testuser")

Puppet.settings.stubs(:value).with(:dbpassword).returns("testpassword")

Puppet.settings.stubs(:value).with(:dbname).returns("testname")
@@ -176,6 +182,7 @@ describe Puppet::Rails, "when initializing a
postgresql connection" do
:adapter => "postgresql",
:log_level => "testlevel",
:host => "testserver",

+ :port => 8192,

:username => "testuser",
:password => "testpassword",
:database => "testname",
@@ -184,10 +191,11 @@ describe Puppet::Rails, "when initializing a
postgresql connection" do
}
end

- it "should provide the adapter, log_level, and host, username,
password, database, connections, and socket arguments" do
+ it "should provide the adapter, log_level, and host, port,
username, password, database, connections, and socket arguments" do

Puppet.settings.stubs(:value).with(:dbadapter).returns("postgresql")

Puppet.settings.stubs(:value).with(:rails_loglevel).returns("testlevel")

Puppet.settings.stubs(:value).with(:dbserver).returns("testserver")

+ Puppet.settings.stubs(:value).with(:dbport).returns(8192)

Puppet.settings.stubs(:value).with(:dbuser).returns("testuser")

Puppet.settings.stubs(:value).with(:dbpassword).returns("testpassword")

Puppet.settings.stubs(:value).with(:dbname).returns("testname")
@@ -198,6 +206,7 @@ describe Puppet::Rails, "when initializing a
postgresql connection" do
:adapter => "postgresql",
:log_level => "testlevel",
:host => "testserver",

+ :port => 8192,

:username => "testuser",
:password => "testpassword",
:database => "testname",