You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@deltacloud.apache.org by mf...@redhat.com on 2012/04/17 15:39:42 UTC
[PATCH core 03/32] Core: Added wrapper for autoloading collections
From: Michal Fojtik <mf...@redhat.com>
Signed-off-by: Michal fojtik <mf...@redhat.com>
---
server/lib/deltacloud/collections.rb | 54 ++++++++++++++++++++++++++++++++++
1 file changed, 54 insertions(+)
create mode 100644 server/lib/deltacloud/collections.rb
diff --git a/server/lib/deltacloud/collections.rb b/server/lib/deltacloud/collections.rb
new file mode 100644
index 0000000..2363887
--- /dev/null
+++ b/server/lib/deltacloud/collections.rb
@@ -0,0 +1,54 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership. The
+# ASF licenses this file to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance with the
+# License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+# License for the specific language governing permissions and limitations
+# under the License.
+
+module Deltacloud
+
+ def self.collection_names
+ @collections.map { |c| c.collection_name }
+ end
+
+ def self.collections
+ @collections ||= []
+ end
+
+ module Collections
+
+ def self.collection(name)
+ Deltacloud.collections.find { |c| c.collection_name == name }
+ end
+
+ def self.deltacloud_modules
+ @deltacloud_modules ||= []
+ end
+
+ Dir[File.join(File::dirname(__FILE__), "collections", "*.rb")].each do |collection|
+ require collection
+ base_collection_name = File.basename(collection).gsub('.rb', '')
+ deltacloud_module_class = Deltacloud::Collections.const_get(base_collection_name.camelize)
+ deltacloud_modules << deltacloud_module_class
+ deltacloud_module_class.collections.each do |c|
+ Deltacloud.collections << c
+ end unless deltacloud_module_class.collections.nil?
+ end
+
+ def self.included(klass)
+ klass.class_eval do
+ Deltacloud::Collections.deltacloud_modules.each { |c| use c }
+ end
+ end
+
+ end
+end
--
1.7.10
Re: [PATCH core 03/32] Core: Added wrapper for autoloading
collections
Posted by Michal Fojtik <mf...@redhat.com>.
On 05/10/12, David Lutterkort wrote:
> On Tue, 2012-04-17 at 15:39 +0200, mfojtik@redhat.com wrote:
> > From: Michal Fojtik <mf...@redhat.com>
>
> [ I am looking at your branch, but it's more convenient to reply to
> individual patches ]
>
> This and 2/32 seem like a little too much magic to me; why do this
> rather than have a big server.rb ? It's 1200 lines right now, but they
> are fairly boring code, and I like being able to see all that code at
> once.
Hi,
Sure not a big deal. I can remove the 'use' in loop and add all collections
to the Deltacloud::API class by hand. I agree that this will make the code more
readable and make the user see what collections are supported by API.
I'll send a patch to do it today.
-- Michal
>
> David
>
>
--
Michal Fojtik
Sr. Software Engineer, Deltacloud API (http://deltacloud.org)
Re: [PATCH core 03/32] Core: Added wrapper for autoloading
collections
Posted by David Lutterkort <lu...@redhat.com>.
On Thu, 2012-05-10 at 10:25 +0200, Michal Fojtik wrote:
> On 05/10/12, David Lutterkort wrote:
> > On Tue, 2012-04-17 at 15:39 +0200, mfojtik@redhat.com wrote:
> > > From: Michal Fojtik <mf...@redhat.com>
> >
> > [ I am looking at your branch, but it's more convenient to reply to
> > individual patches ]
> >
> > This and 2/32 seem like a little too much magic to me; why do this
> > rather than have a big server.rb ? It's 1200 lines right now, but they
> > are fairly boring code, and I like being able to see all that code at
> > once.
>
> Sorry, I was too fast with replying ;-) You are referring to the collection
> subfolder not to lazy-loading.
>
> My thoughts was to make it more readable and structured when it will be
> splitted into separate files.
After looking at it again, I am fine with splitting it across multiple
files; I mostly object to the lazy-loading stuff. IMHO, it makes the
code less readable: when I open collection.rb, I expect to see a bunch
of 'require' statements and not much else; instead I need to actually
read through the code now and figure out that it essentially does what
the require's would do.
And having explicit 'use' in server.rb would make things more readable,
too.
David
Re: [PATCH core 03/32] Core: Added wrapper for autoloading
collections
Posted by Michal Fojtik <mf...@redhat.com>.
On 05/10/12, David Lutterkort wrote:
> On Tue, 2012-04-17 at 15:39 +0200, mfojtik@redhat.com wrote:
> > From: Michal Fojtik <mf...@redhat.com>
>
> [ I am looking at your branch, but it's more convenient to reply to
> individual patches ]
>
> This and 2/32 seem like a little too much magic to me; why do this
> rather than have a big server.rb ? It's 1200 lines right now, but they
> are fairly boring code, and I like being able to see all that code at
> once.
Sorry, I was too fast with replying ;-) You are referring to the collection
subfolder not to lazy-loading.
My thoughts was to make it more readable and structured when it will be
splitted into separate files.
Having everything in one place is nice but when you first look into the
server.rb it honestly looks like a mess :-)
IMHO having the collections in separate files will make it more easy to fix
things in particular collection, without making people rebasing and fixing
conflicts in one big server.rb.
Also since this code use Base class from Sinatra, all collections are just
'modules' that can be plugged and un-plugged as needed. So you can for
example use DC as base for your application where you choose just
collections you really want to use.
Another thing is that I guess in future we will need to add more
collections, like for managing networks/etc so the amount of code in
server.rb will eventually grow. I think that for the people that want to
add new collection to Deltacloud, it's now more easy just 'copy' the
collection file, rename it and add their stuff there.
-- Michal
--
Michal Fojtik
Sr. Software Engineer, Deltacloud API (http://deltacloud.org)
Re: [PATCH core 03/32] Core: Added wrapper for autoloading
collections
Posted by David Lutterkort <lu...@redhat.com>.
On Tue, 2012-04-17 at 15:39 +0200, mfojtik@redhat.com wrote:
> From: Michal Fojtik <mf...@redhat.com>
[ I am looking at your branch, but it's more convenient to reply to
individual patches ]
This and 2/32 seem like a little too much magic to me; why do this
rather than have a big server.rb ? It's 1200 lines right now, but they
are fairly boring code, and I like being able to see all that code at
once.
David