8
\$\begingroup\$

I am trying to load a module according to some settings. I have found a working solution but I need a confirmation from an advanced python developer that this solution is the best performance wise as the API endpoint which will use it will be under heavy load.

The idea is to change the working of an endpoint based on parameters from the user and other systems configuration. I am loading the correct handler class based on these settings. The goal is to be able to easily create new handlers without having to modify the code calling the handlers.

This is a working example:

./run.py:

from flask import Flask, abort 
import importlib 
import handlers  

app = Flask(__name__)  

@app.route('/') 
def api_endpoint():     
    try:         
        endpoint = "simple" # Custom logic to choose the right handler        
        handlerClass = getattr(importlib.import_module('.'+str(endpoint), 'handlers'), 'Handler')         
        handler = handlerClass()     
    except Exception as e:         
        print(e)         
        abort(404)      

    print(handlerClass, handler, handler.value, handler.name())      

    # Handler processing. Not yet implemented

    return "Hello World"  

if __name__ == "__main__":     
    app.run(host='0.0.0.0', port=8080, debug=True)

One "simple" handler example. A handler is a module which needs to define an Handler class:

./handlers/simple.py:

import os  

class Handler:     
    def __init__(self):         
        self.value = os.urandom(5)      

    def name(self):        
        return "simple"

If I understand correctly, the import is done on each query to the endpoint. It means IO in the filesystem with lookup for the modules, ...

Is it the correct/"pythonic" way to implement this strategy?

The answer so far from the previous Stack Overflow exchange:

I would whitelist if possible.

I already whitelist when I fill in the endpoint variable with my custom logic.

It does not seem to be a very clean solution. Each handler needs to define an Handler class (same name for different class). I am searching for a cleanest "extension mecanism". How would you clean it up?

Based on @netme answer, this is the 2.0 version

\$\endgroup\$
6
\$\begingroup\$

the idea is nice, but I have few concerns. Firs of all, trying to call importlib every time you are accessing the API endpoint is a bit expensive. It will be a way better if you will introduce some cache first.

You can use 2 approaches to build that cache:

  • Lazy, where you lookup cache first and if you cannot find the proper handler, you are trying to load it using your trick with importlib
  • The standard approach where you can use e.g. pkgutil.iter_modules (https://docs.python.org/2/library/pkgutil.html#pkgutil.iter_modules) and build the handler cache before you are actually start handling the API endpoints.

I have also few notes regarding your code:

  1. Use format functions instead of concatenating strings:

    ".{}".format(endpoint)
    
  2. Don't catch general exceptions (just use more specific classes than Exception class) and try not to put more then 1 statement under the try...except block.

  3. Use pythonic naming handlerClass -> handler_class

  4. In your handlers you can make names as read-only properties:

    @property
    def name(self):
        return "sample"
    
  5. Try to follow PEP8 in module imports:

    # System modules
    import importlib 
    
    # 3rd party
    from flask import Flask, abort 
    
    # your modules
    import handlers  
    
\$\endgroup\$
  • \$\begingroup\$ Thank you for your detailed answer. Why is format function better than concatenation ? You speak about caching module. But is it not a standard feature of the python import mechanism ? \$\endgroup\$ – jobou Jul 10 '15 at 22:43
  • \$\begingroup\$ @jobou, the format function is more readable. I'm trying to use it everywhere in my code. \$\endgroup\$ – Mikhail Chernykh Jul 11 '15 at 9:46
  • 1
    \$\begingroup\$ Regarding caching, you can introduce a local dict where you cache it. All you need is to add _endpoints_cache dict and then if your endpoint is not in cache you should use importlib to import it once and then cache it _endpoints_cache["simple"] = handler (sorry it's a bit problematic to put code samples here) \$\endgroup\$ – Mikhail Chernykh Jul 11 '15 at 9:52
  • \$\begingroup\$ I have made a 2.0 version of the code based on your answer (cf link in the original post). Can you give me your opinion ? \$\endgroup\$ – jobou Jul 11 '15 at 20:01
  • \$\begingroup\$ @jobou, looks much better now! I can agree with Janos answer regarding PEP8 but in general I like the new version of your code. \$\endgroup\$ – Mikhail Chernykh Jul 12 '15 at 11:55

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.