geopython / pygeoapi

pygeoapi is a Python server implementation of the OGC API suite of standards. The project emerged as part of the next generation OGC API efforts in 2018 and provides the capability for organizations to deploy a RESTful OGC API endpoint using OpenAPI, GeoJSON, and HTML. pygeoapi is open source and released under an MIT license.
https://pygeoapi.io
MIT License
499 stars 262 forks source link

Why Api is already rendering to JSON/HTML #1802

Open vvmruder opened 1 month ago

vvmruder commented 1 month ago

Is your feature request related to a problem? Please describe.

It is not a problem but an architectural question. And before I put too much effort in this I wanted to ask here. I work on a project which like to integrate pygeoapi for at least the OAPIF part. Reviewing the current code, I wonder about the level where actual rendering of results takes place. We can take get_collection_queryables as an example. But it is valid for all API endpoints.

In my opinion, rendering to json/html already here is a bit too early since it blocks an integrating app from hooking into the process to inject specific things. Maybe we want to add extra links, which are app specific. Or apply additional filters on the result set before it is delivered as response. Currently one option I do have, is to subclass the API , overwrite the desired function (where I have probably 99% copied from the original one). The second option is to load responded JSON to dict again in the calling method of my project to manipulate it and then redump it to json again. Both solutions are not really sufficient. The first not because it will introduce undeniable problems when it comes to upgrading pygoapi over time. The second is a perfomance killer and especially for the OAPIF not a real option.

In my opinion rendering should be left to the actual framework (django, flask, etc) completely. Instead of solving rendering in the API of pygeoapi we should invest a bit to correctly type the interface of the responses (with eg. dataclasses).

I kindly offer my support to help with that if this idea is accepted.

Describe the solution you'd like It can be solved by completely getting rid of any Jinja render or JSON dump logic at the API class levels. Instead this should be a task for the framework actually producing it. I can imagine that we still could offer flask and django default apps as it is done today. But with a bit more logic in these dedicated methods (e.g. django collection_queryables). The rendering should teka place in these methods.

Describe alternatives you've considered Alternatives are the 2 approaches I described in the problem description.

Additional context no addional context

m-kuhn commented 4 weeks ago

@tomkralidis @webb-ben @kalxas any opinions on this proposal of separating API logic from rendering?

totycro commented 2 weeks ago

I wasn't present when the initial implementation was started, but from what i understood from working with pygeoapi, the intention was to have as little web framework specific code as possible. I agree that it's' not a common architecture that the API implementation already renders the data to json, but it does make the flask/django/starlette adapter straightforward. Therefore updates of those web frameworks are usually trivial, however at the cost that the API implementations has to do this low level work.

This makes pygeoapi hard to embed as a library as described above, but i guess the usual way of embedding it would be to include the web framework specific code as views or as an app.

If we were to change this, it would touch a lot of code and it might not be easy to get all cases right. If we want pygeoapi to be callable by other python code and not only by embedding the API views, i think it's necessary though.