pygeoapi icon indicating copy to clipboard operation
pygeoapi copied to clipboard

Plugin Architecture: Formatters and Dependency Injection via Factories

Open geekdenz opened this issue 4 years ago • 10 comments

Problem Writing code for plugins is very difficult, because a lot of the code has to live in common files and this causes PRs to be harder to do.

Solution Decouple plugins from the core architecture by registering plugins with the correct module. This can be achieved by having a simple register function inside of the module that can have a plugin. For example, a formatter plugin would be registered like this:

plugins/geopackage.py:

from pygeoapi.formatter.formatters import register
class GeopackageFormatter:
  def handle(*params): # could be a well defined interface for a formatter
    pass
register('geopackage', GeopackageFormatter) # formatter specific register function
# with format as the parameter and the class implementing the formatter specific 
# to this format

Users can define which plugins are loaded in configuration config.yml:

server:
    plugins:
        - geopackage

See for Proof-of-Concept Details https://github.com/geekdenz/pygeoapi/commit/afe07a8cfebf9271dd1d005005ea57cac5874c08

Old Info for Reference We can solve this by refactoring plugins into separate files and have decorators essentially call the plugins. An example for formatters is here: https://github.com/geekdenz/pygeoapi/commit/3d71c30e2283cd9b4ddf56e746623ab9f4430bc9 I tried it and it works. Security can be enforced by sanitising the input, i.e. the format input string in this case to only allow alphabetical characters with a regular expression.

Describe alternatives you've considered Add decorator for each plugin. This works for JSON-LD nicely as it only adds one extra line to the api.py file. This is not quite as elegant, but has the advantage of being easier to understand and implement.

Not using decorators is faster, but since we're using a dynamic language, this O(1) addition to execution time is well-worth the benefit of easier maintainance.

Additional context I got frustrated merging in the GeoPackage changes I did and couldn't see how this would work nicely and future proof. We could just add a plugins folder where the plugins live as simple python scripts. The plugins folder could have subfolders (modules) such as formatter etc..

geekdenz avatar Apr 30 '20 08:04 geekdenz

@geekdenz can you elaborate with an example a little bit the hard part to write plugins? Sorry but I cannot figure out it since the injection of plugins looks like well architected

francbartoli avatar Apr 30 '20 09:04 francbartoli

@francbartoli thanks for your reply. Maybe I don't understand the plugin injection part very well. Before I elaborate, could you describe how the injection works? Is this DI? If so, it might be better than what I did. I would be quite happy to understand how that works.

geekdenz avatar Apr 30 '20 09:04 geekdenz

Just had a look at api.py and yes, I agree the plugin architecture is good and simple. But it doesn't quite work with the formatter plugin type, because the user will have to provide a format that is currently limited by the source code in api.py. Maybe we could have this as part of a formatter decorator or simply as a function call in the formatter include, such as:

## csv_.py
import provide_formatter from pygeoapi.format...

class CSVFormatter(BaseFormatter):
    """CSV formatter"""
    # maybe better in the constructor
    def __init__(self, formatter_def, format_, mime):

# or as a function call
provide_formatter('csv', 'text/csv', CSVFormatter) # or whatever the mime-type for csv is...

for my example.

This is to minimise the amount of messing around with core code when contributing a plugin.

geekdenz avatar Apr 30 '20 09:04 geekdenz

This latter could be interesting, can you reword the issue and its description to be focused on this?

francbartoli avatar Apr 30 '20 10:04 francbartoli

Hi @geekdenz thanks for the report. Should we update our formatting plugins setup to how we do plugins for providers and processes, which solves the issue of touching many files in the codebase:

  • for core plugins, add plugin and register in pygeoapi/plugins.py
  • for non-core plugins add to Python path and integrate into configuration with a factory approach

The formatter support is admittedly half-baked right now and needs a refactor to align accordingly.

tomkralidis avatar Apr 30 '20 11:04 tomkralidis

OK. @francbartoli I'll try another proof-of-concept of this the function call way. I wonder if it would be even more elegant with dependency injection.

geekdenz avatar Apr 30 '20 11:04 geekdenz

@geekdenz yep I'd like to have DI but also agree with @tomkralidis to adopting the same strategy of plugins for providers and processes, it will be much more coherent

francbartoli avatar May 02 '20 11:05 francbartoli

Also @geekdenz, can you change the title accordingly?

francbartoli avatar May 02 '20 11:05 francbartoli

@francbartoli Does the title make sense?

geekdenz avatar May 04 '20 01:05 geekdenz

Hey @geekdenz, any update on this? I ended up to this https://python-dependency-injector.ets-labs.org/examples/flask.html that would be relevant IMHO

francbartoli avatar Mar 12 '21 14:03 francbartoli

As per RFC4, this Issue has been inactive for 90 days. In order to manage maintenance burden, it will be automatically closed in 7 days.

github-actions[bot] avatar Mar 10 '24 21:03 github-actions[bot]

As per RFC4, this Issue has been closed due to there being no activity for more than 90 days.

github-actions[bot] avatar Mar 24 '24 03:03 github-actions[bot]