[albatross-users] page module import magic [PATCH included]

Andrew McNamara andrewm at object-craft.com.au
Tue Jan 13 20:24:04 EST 2004


>Hierarchies of pages do work but not quite correctly in the released 
>code base, hence my original bug report. The only problem with them is 
>the corruption of sys.modules. Perhaps Albatross was never really meant 
>to be used like this?

There was code to explicitly support pages with names like "foo/bah", so I
think Dave's intention was to support them. I think Dave's implementation
would have failed when a sub-page module contained a class definition, an
instance of which appeared in the context (did that make sense? The module
path was wrong, so it couldn't restore the class definition on unpickle).

>For me, using a '.' for this purpose in a URL is odd. URLs are designed 
>to map to resources (real or virtual) so using a '.' is creating your 
>own URL scheme when a normal URL would have been good enough. Hmm ... is 
>there a problem with simply converting a URL of preferences/simple to a 
>module called preferences.simple?

I replied to this earlier in the day (to me it's a module name, rather
than a URL) - but in the latest version of the patch (attached), I do
replace('/', '.'), so either will work.

>>get_module_holder() just makes sure the dummies are created (recursively)
>>and returns the parent of the module to be loaded.
>
>Ah, ok. I understand now. Would this work if I wanted the URL 
>"preferences" *and* "preferences/simple" to work correctly or would the 
>dummy __alpage__.preferences module get in the way?

The latest version of the patch copes with this also.

>I actually thought your preferred solution was to use *real* application 
>packages and modules (which I guess is why I was a little confused about 
>your "dummy parent modules" comment) but I've reread the thread and 
>found mention of a "synthetic namespace". 

My feeling is that there are fundemental differences between python
packages and directories containing page modules - in particular,
the coupling between modules in a python package is tighter, whereas a
directory full of page modules is just an organisational thing. I also
don't like the fact that existing users will have to go around dropping
useless __init__.py files everywhere.

There are additional issues caused by the package namespace rules... so
if you have a module A.B.C, then getattr(A.B, 'C') should return module
C by normal python rules. It seems wrong that page module A.B should
"contain" page module A.B.C. It also seems wrong that loading page module
A.B.C should load A and A.B as well.

>>That pretty much describes the changes - does it make more sense now?
>>
>Yes, that's great. Thanks.

As always, if what I say doesn't make sense, tell me. 8-)

>I would be grateful if you could keep me updated on the solution you 
>choose. I would need to change my code in a number of places to use a 
>'.' in the URL but it's probably not too big a problem and I would 
>rather use an official Albatross release than my patched version. 
>However, nice URLs will be important to me for some future projects.

Try this version if you do...

Index: app.py
===================================================================
RCS file: /usr/local/cvsroot/object-craft/albatross/albatross/app.py,v
retrieving revision 1.87
diff -u -d -r1.87 app.py
--- app.py	9 Jan 2004 12:38:27 -0000	1.87
+++ app.py	13 Jan 2004 09:07:59 -0000
@@ -7,7 +7,6 @@
 
 import os
 import sys
-import stat
 import imp
 import urlparse
 
@@ -374,10 +373,11 @@
     '''Caching module loader
     '''
 
+    mod_holder_name = '__alpage__'
+
     def __init__(self, base_dir, start_page):
         self.__base_dir = base_dir
         self.__start_page = start_page
-        self.__cache = {}
 
     def module_path(self):
         return self.__base_dir
@@ -393,35 +393,52 @@
             ctx.set_page(name)
         self.load_page_module(ctx, name)
 
+    def is_page_module(self, name):
+        return name.startswith(self.mod_holder_name)
+
     def load_page_module(self, ctx, name):
-        if self.__base_dir == '.':
-            path = name
+        mod_path = name.replace('/', '.').split('.')
+        if mod_path[0] != self.mod_holder_name:
+            mod_path.insert(0, self.mod_holder_name)
+        # Create module path out of dummy modules, if needed
+        for i in range(len(mod_path) - 1, 0, -1):
+            mod_name = '.'.join(mod_path[:i])
+            try:
+                sys.modules[mod_name]
+                break
+            except KeyError:
+                sys.modules[mod_name] = imp.new_module(mod_name)
+        # Now see if it's already loaded
+        mod_name = '.'.join(mod_path)
+        try:
+            module = sys.modules[mod_name]
+        except KeyError:
+            # Nope
+            pass
         else:
-            path = os.path.join(self.__base_dir, name)
-        page = self.__cache.get(path)
-        if page:
-            mtime = os.stat(page.__file__)[stat.ST_MTIME]
-            if mtime > page.__mtime__:
-                reload(page)
-                page.__mtime__ = mtime
-        if not page:
-            dirname, name = os.path.split(path)
-            file, filepath, desc = imp.find_module(name, [dirname])
-            page = sys.modules.get(name)
-            if not page or not page.__file__.startswith(filepath):
-                try:
-                    page = imp.load_module(name, file, filepath, desc)
-                finally:
-                    if file:
-                        file.close()
-            if not (hasattr(page, 'page_enter') or
-                    hasattr(page, 'page_leave') or
-                    hasattr(page, 'page_process') or
-                    hasattr(page, 'page_display')):
-                raise ApplicationError('module "%s" does not define one of page_enter, page_leave, page_process or page_display' % (name))
-            page.__mtime__ = os.stat(page.__file__)[stat.ST_MTIME]
-            self.__cache[path] = page
-        ctx.page = page
+            # The name exists, but is it a dummy or the real thing?
+            if hasattr(module, '__file__'):
+                # Real? Then return it
+                ctx.page = module
+                return module
+            # Dummy? but we want the real thing... so continue
+        mod_dir = os.path.join(self.__base_dir, *mod_path[1:-1])
+        try:
+            f, filepath, desc = imp.find_module(mod_path[-1], [mod_dir])
+        except ImportError, e:
+            raise ApplicationError('%s (in %s)' % (e, mod_dir))
+        try:
+            module = imp.load_module(mod_name, f, filepath, desc)
+        finally:
+            if f:
+                f.close()
+        if not (hasattr(module, 'page_enter') or
+                hasattr(module, 'page_leave') or
+                hasattr(module, 'page_process') or
+                hasattr(module, 'page_display')):
+            raise ApplicationError('module "%s" does not define one of page_enter, page_leave, page_process or page_display' % (name))
+        sys.modules[mod_name] = ctx.page = module
+        return module
 
     def page_enter(self, ctx, args):
         if hasattr(ctx.page, 'page_enter'):
@@ -451,8 +468,8 @@
         self.__start_page = start_page
         self.__page_objects = {}
 
-    def module_path(self):
-        pass
+    def is_page_module(self, name):
+        return False
 
     def start_page(self):
         return self.__start_page
Index: context.py
===================================================================
RCS file: /usr/local/cvsroot/object-craft/albatross/albatross/context.py,v
retrieving revision 1.93
diff -u -d -r1.93 context.py
--- context.py	21 Sep 2003 04:40:31 -0000	1.93
+++ context.py	13 Jan 2004 09:08:00 -0000
@@ -10,6 +10,7 @@
 import stat
 import re
 import cPickle
+import __builtin__
 
 try:
     import zlib
@@ -564,9 +565,13 @@
         self.clear_locals()
 
     def decode_session(self, text):
-        module_path = self.app.module_path()
-        if module_path:
-            sys.path.insert(0, module_path)
+        def imp_hook(name, globals, locals, fromlist):
+            if self.app.is_page_module(name):
+                return self.app.load_page_module(self, name)
+            else:
+                return real_imp(name, globals, locals, fromlist)
+
+        real_imp, __builtin__.__import__ = __builtin__.__import__, imp_hook
         try:
             try:
                 vars = cPickle.loads(text)
@@ -577,8 +582,7 @@
             for name in vars.keys():
                 self.__vars[name] = 1
         finally:
-            if module_path:
-                sys.path.remove(module_path)
+            __builtin__.__import__ = real_imp
 
     def encode_session(self):
         vars = {}

-- 
Andrew McNamara, Senior Developer, Object Craft
http://www.object-craft.com.au/



More information about the Albatross-users mailing list