Extension pattern in a flask controller using importlib 2.0

Question

The first edition of the code can be found in this 1.0 version.

To summarize the problem :

• I have an endpoint in flask where the work logic change according to the query
• I am trying to implement an extension mechanism where the right class is loaded
• The first version did not provide caching of the imported class

Based on the answer from @netme, this is a code update where I lazy load the right handler class and cache it :

./run.py:

from flask import Flask, abort
import handler
app = Flask(__name__)
handler_collection = handler.HandlerCollection()
@app.route('/')
def api_endpoint():
 try:
 endpoint = "simple" # Custom logic to choose the right handler
 handler_instance = handler_collection.getInstance(endpoint)
 except handler.UnknownEndpoint as e:
 abort(404)
 print(handler_instance, handler_instance.name)
 # Handler processing. Not yet implemented
 return "Hello World"
if __name__ == "__main__":
 app.run(host='0.0.0.0', port=8080, debug=True)

./handler.py:

# -*- coding: utf-8 -*-
import importlib
import handlers
class UnknownEndpoint(Exception):
 pass
class HandlerCollection:
 _endpoints_classes = {}
 def addClass(self, endpoint_class):
 self._endpoints_classes[endpoint_class.name] = endpoint_class
 def getClass(self, endpoint_name):
 if (endpoint_name in self._endpoints_classes):
 return self._endpoints_classes.get(endpoint_name)
 try:
 # Try to import endpoint handler from a module in handlers package
 endpoint_module = importlib.import_module(
 '.{}'.format(str(endpoint_name)),
 'handlers')
 endpoint_module.register(self)
 except ImportError as e:
 raise UnknownEndpoint('Unknown endpoint \'{}\''.format(endpoint_name)) from e
 return self._endpoints_classes.get(endpoint_name)
 def getInstance(self, endpoint_name):
 endpoint_class = self.getClass(endpoint_name)
 return endpoint_class()

./handlers/simple.py:

# -*- coding: utf-8 -*-
class SimpleWebhookHandler:
 name = "simple"
def register(handler_collection):
 handler_collection.addClass(SimpleWebhookHandler)

Each handler needs to provide a register function which allows to have different handler class name and each handler class needs to provide a class attribute name.

Can you give me your opinion on this piece of code ? Is it "pythonic" ?

Answer 1

Can you give me your opinion on this piece of code ? Is it "pythonic" ?

I don't really see anything non-pythonic about it. Some minor coding style issues though:

• Minor PEP8 violations:
  • put 2 blank lines before class and top-level function definitions
  • use snake_case for function names instead of camelCase
• Unnecessary parentheses in if (endpoint_name in self._endpoints_classes):

A bigger issue I see is that the module loading is convoluted and fragile:

• HandlerCollection.getClass tries to import a module : violates the single responsibility principle, it would be better to move this logic somewhere else
• The imported module is expected to have a register function : a semantic rule, not obvious enough from the code itself. There are no docstrings either, so as it is, implementors have to dig this piece of information out of the current implementation details
• The register function must pass a class that has a name property defined : semantic rule, see the previous item

• The logical flow of HandlerCollection.getClass is convoluted:

  1. try to import module if missing
  2. pass self to module.register
  3. module.register calls me.addClass

  I suggest to move the module loading part out to a different method, perhaps HandlerRegistry.load_and_register_if_missing, and let getClass behave as a simple cache.

• jobou: I don't understand. The 2 first line of the getClass method are doing your first note on endpoint_name. If you look at the register function in the module handlers.simple, it registers the handler class in the collection so it won't be None. Thanks for pointing out the PEP issues.