On 02/04/2011 08:30 AM, Scott Seago wrote:
Hi,
Overall this seems to work fairly well -- I did run into a few issues
though:
Hi Scott, thanks for feedback.
On 02/02/2011 07:58 AM, jprovazn(a)redhat.com wrote:
> From: Jan Provaznik<jprovazn(a)redhat.com>
>
> Realm = backend realm provided by a provider
> FrontendRealm = conductor realm (actually only name)
>
> Frontend realm can be mapped to many provider realms or providers.
> Provider
> realm or provider can be mapped to many frontend realms. User defines
> this
> mapping in UI.
At a high level, I don't see anything relating to Condor here. I'm
assuming that the condor classads generation will need to be updated to
take into account the new mappings and rules. Have you looked into this
or discussed it with Ian? I'm fairly sure the condor changes will be
fairly easy, but they'll need to be done.diff --git
a/src/db/migrate/20110126121114_frontend_realms.rb
b/src/db/migrate/20110126121114_frontend_realms.rb
Not yet, my plan was to wait till this patch is ACKed and then discuss
required changes with condor guys (think it's better for them if they
have finished patch on which they can build on). But when reading
condormatic_instance_create it really seems to be easy, so I can do
first shot.
> diff --git a/src/app/models/frontend_realm.rb
> b/src/app/models/frontend_realm.rb
> new file mode 100644
> index 0000000..ce266a7
> --- /dev/null
> +++ b/src/app/models/frontend_realm.rb
> @@ -0,0 +1,33 @@
> +#
> +# Copyright (C) 2009 Red Hat, Inc.
> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; version 2 of the License.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program; if not, write to the Free Software
> +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
> +# MA 02110-1301, USA. A copy of the GNU General Public License is
> +# also available
athttp://www.gnu.org/copyleft/gpl.html.
> +
> +# Filters added to this controller apply to all controllers in the
> application.
> +# Likewise, all the methods added will be available for all controllers.
> +
> +class FrontendRealm< ActiveRecord::Base
> + has_many :realm_backend_targets
> + has_many :instances
> +
> + # there is a problem with has_many through + polymophic in AR:
> + #http://blog.hasmanythrough.com/2006/4/3/polymorphic-through
> + # so we define explicitly backend_realms and backend_providers
> + has_many :backend_realms, :through => :realm_backend_targets,
> :source => :realm, :conditions =>
> 'realm_backend_targets.realm_or_provider_type = "Realm"'
> + has_many :backend_providers, :through => :realm_backend_targets,
> :source => :provider, :conditions =>
> 'realm_backend_targets.realm_or_provider_type = "Provider"'
> +
The above two lines fail when the database is postgres, since with PG,
you have to use single-quote (') to delimit strings, rather than
double-quote ("). Change it like this:
Ouch, dammit sqlite, I didn't want to loose my existing postgres data
and was lazy to create another postgres db for this patch :/.
diff --git a/src/app/models/frontend_realm.rb
b/src/app/models/frontend_realm.rb
index ce266a7..33e51f2 100644
--- a/src/app/models/frontend_realm.rb
+++ b/src/app/models/frontend_realm.rb
@@ -26,8 +26,8 @@ class FrontendRealm < ActiveRecord::Base
# there is a problem with has_many through + polymophic in AR:
#
http://blog.hasmanythrough.com/2006/4/3/polymorphic-through
# so we define explicitly backend_realms and backend_providers
- has_many :backend_realms, :through => :realm_backend_targets, :source
=> :realm, :conditions => 'realm_backend_targets.realm_or_provider_type
= "Realm"'
- has_many :backend_providers, :through => :realm_backend_targets,
:source => :provider, :conditions =>
'realm_backend_targets.realm_or_provider_type = "Provider"'
+ has_many :backend_realms, :through => :realm_backend_targets, :source
=> :realm, :conditions => "realm_backend_targets.realm_or_provider_type
= 'Realm'"
+ has_many :backend_providers, :through => :realm_backend_targets,
:source => :provider, :conditions =>
"realm_backend_targets.realm_or_provider_type = 'Provider'"
validates_presence_of :name
end
> + validates_presence_of :name
> +end
> new file mode 100644
> index 0000000..291a3ba
> --- /dev/null
> +++ b/src/db/migrate/20110126121114_frontend_realms.rb
> @@ -0,0 +1,24 @@
> +class FrontendRealms< ActiveRecord::Migration
> + def self.up
> + drop_table :realm_map
> + create_table :frontend_realms do |t|
> + t.string :name, :null => false, :limit => 1024
> + end
You should add these to the model too:
t.integer :lock_version, :default => 0
t.timestamps
ok
The UI worked for me, but I had one comment -- in the realms dropdown,
you show the realm name but not the provider name. Since it's possible
that 2 providers will have the same realm names, the dropdown list
should probably be in the form "Providername: Realmname" instead of
simply "Realmname"
ok
Also, the following change (from patch 4/5) caused spec test
failures:
diff --git a/src/app/controllers/admin/users_controller.rb
b/src/app/controllers/admin/users_controller.rb
index 58aabf8..0401745 100644
--- a/src/app/controllers/admin/users_controller.rb
+++ b/src/app/controllers/admin/users_controller.rb
@@ -61,7 +61,7 @@ class Admin::UsersController< ApplicationController
end
render :partial => @details_tab
end
- format.html { render :partial => @details_tab }
+ format.html { render :action => 'show'}
end
end
Was this bit checked in by accident? It doesn't seem to be related to
realms, nor does it seem like a correct change.
this shouldn't be there, accident
Scott
Jan