[openstack-dev] [Keystone] Refactoring the routers, controllers and core for modules
Adam Young
ayoung at redhat.com
Wed Dec 12 03:27:33 UTC 2012
Here is a large refactoring I have submitted for code review.
https://review.openstack.org/#/c/17782/ This was kicked off by a
problem I found while working on Trusts.
I think we are good on most things, but the sticking point is where to
put the controllers, and how to name them. We can see the problem if we
limit talking about identity and token. If we solve that, policy and
catalog will fall into line.
So, if we move the token controllers into keystone/token/ and reference
them in __init__.py we get a circular dependency. Why?
identity.*Controller needs token Manager and Driver. so it imports
from keystone import token
from keystone.identity import core
token.*Controller needs identity Manager and Driver.
from keystone import identity
from keystone.token import core
so we have the following chain.
identity/controllers.py -> token/__init__.py -> token/controllers.py ->
identity/__init__.py -> identity/controllers.py
They not only import each other, but they attempt to add the imported
namespace into their GLobals collection. So you get
identity.token.identity.token.identity.token.identity.token.identity.token.identity.token.identity.token.identity
Which manifests itself as "Cannot import token..."
The reason we haven't seen this so far is because identity and token
were set up very differently. Controllers for Token were in service.py.
Dolph's major objections, if I understand correctly, are best seen in
https://review.openstack.org/#/c/17782/7/keystone/contrib/ec2/core.py
fromkeystone.token importcontrollers astoken_controllers
Which gives us:
token_controller =token_controllers.TokenController()
We would both rather be able to do:
token_controller = token.controllers.Token()
But to enable that would require the __init_.py import that would then
trigger the circular dependency.
Now, we could put the controllers in their own module. I had thought
about having one major package for routers, one for controllers, and one
for the stuff in the core files. I'd probably call that last "domain"
or "data." But I don't think it gains us much, and it is yet another
parallel structure.
Here is the general scheme of the refactoring.
1. Only ComposingRouters are in service.py. These will only know about
ComposableRouters.
(for the uninitiated, the ComposingRouters are designed to put in a
bunch of ComposableRouters to build one big routing table per WSGI App.)
2. ComposableRouters need to live in a namespace that identifies the
domain that they solve; identity routers need to be in a namespace
called identity and so forth. I have them in individual files
underneath keystone/routers. So my import for service.py looks like this:
from keystone.routers import identity
from keystone.routers import policy
from keystone.routers import catalog
from keystone.routers import token
The code that creates the ComposableRouters only imports the controllers
for a single module. Thus the imports for routers/identity.py looks like
from keystone.common import wsgi
from keystone.identity import controllers
The controllers right now are in keystone/<namespace>/controllers.py but
are not referenced by keystone/<namespace>/__init__.py
So the question is what owns the "good namespace" for the module. When
I import keystone/identity, should I get the Manager and Driver, or
should I get the Controllers. It can't be both.
I think the current setup is the cleanest separation. I was originally
going to go the other way, with the controller owning the good
namespace, but too many things reference the core Classes directly:
1. Most controllers need to know about multiple core objects.
2. All of the contrib code accesses the core objects directly
3. Many of the unit tests import the core objects
It seems like the only thing that needs the controllers are the routers,
and a little bit of contrib code. We should probably be writing unit
tests against the controllers. We have them for service.py
(TokenController) but most of the rest of the Controller code is
exercised via more functional tests.So, to summarize, I propose:
1 service.py contains only ComposingRouters and only imports from
keystone/routers
2 keystone.routers contains one file permodule, named after that module,
that imports the controllers file from that module.
3. Each module directory contains controllers.py which can point to any
of the modules to reference the core classes.
4. __init__.py for each module will only from <module>.core import *
Sorry so long. I'm trying to lay out the whole process that lead to the
decision. Hopefully, this at least helps illuminate the Keystone code.
More information about the OpenStack-dev
mailing list