Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GNIP-79: GeoNode REST APIs (v2) #6685

Closed
3 of 5 tasks
afabiani opened this issue Dec 1, 2020 · 11 comments · Fixed by #6686
Closed
3 of 5 tasks

GNIP-79: GeoNode REST APIs (v2) #6685

afabiani opened this issue Dec 1, 2020 · 11 comments · Fixed by #6686
Assignees
Labels
gnip A GeoNodeImprovementProcess Issue
Milestone

Comments

@afabiani
Copy link
Member

afabiani commented Dec 1, 2020

GNIP-79: GeoNode REST APIs (v2)

Overview

Create a real REST API for GeoNode resources, which is:

  • Pluggable: new external dependencies and modules should be easily allowed to add their own API endpoints without modifying the GeoNode code base.
  • Modular: each app has their own API module if needed. No more monolithic modules with a ton of classes
  • REST compliant: the new API must adhere to the REST paradigm and protocol
  • Live together with the standard Tastypie based GeoNode APIs

Proposed By

@afabiani
@giohappy

Assigned to Release

This proposal is for GeoNode 3.2.

State

  • Under Discussion
  • In Progress
  • Completed
  • Rejected
  • Deferred

Motivation

We would like to transition out from old Tastypie based GeoNode APIs, or, at least, provide a more lightweight, REST compliant and secure set of API endpoints.

The proposal is to collect all those endpoints under the /api/v2 URI prefix.

Proposal

Define a common REST router under geonode.api

router = routers.DynamicRouter()

schema_view = get_schema_view(
    openapi.Info(
        title="GeoNode REST API",
        default_version='v2',
        description="Application for serving and sharing geospatial data",
        terms_of_service="https://github.com/GeoNode/geonode/wiki/Community-Bylaws",
        contact=openapi.Contact(email="dev@geonode.org"),
        license=openapi.License(name="GPL License"),
    ),
    public=True,
    permission_classes=(permissions.AllowAny,),
)

Define APIs for each GeoNode module

As an instance for geonode.base.api we can easily plug ResourceBase and Users routes as follows:

image

from geonode.api.urls import router

from . import views

router.register(r'users', views.UserViewSet)
router.register(r'groups', views.GroupViewSet)
router.register(r'base_resources', views.ResourceBaseViewSet)

Backwards Compatibility

NO Backwards Compatible.

Future evolution

  • Get rid of Tastypie
  • Allow GeoNode to be shipped in two modules, pure models/APIs and frontend

Feedback

Update this section with relevant feedbacks, if any.

Voting

Project Steering Committee:

  • Alessio Fabiani: 👍
  • Francesco Bartoli:
  • Giovanni Allegri:👍
  • Simone Dalmasso:
  • Toni Schoenbuchner: 👍
  • Florian Hoedt: 👍

Links

@afabiani afabiani added the gnip A GeoNodeImprovementProcess Issue label Dec 1, 2020
@afabiani afabiani self-assigned this Dec 1, 2020
@t-book
Copy link
Contributor

t-book commented Dec 1, 2020

absolutely +1!

@afabiani afabiani added this to the 3.2 milestone Dec 1, 2020
@gannebamm
Copy link
Contributor

I like the idea and current dev implementation. Nonetheless we should discuss how and if this will connect and/or interfere with pygeoapi.

@francbartoli
Copy link
Member

I like the general idea to get rid of the old API system. However, I have some concern about how this has been tackled. In particular as far as I can see:

  • The data model is driven by the Django models rather than being OpenAPI centric with a document defining, at the least, the minimum viable product for a new GeoNode Core API system in terms of endpoints, resources, parameters, responses, each of them with their own schema to enforce the validation eventually. I understood this has been decided to preserve the retro-compatibility. In other words, the OpenAPI document (formerly swagger) is just the output of the ORM at the moment. It would be great to have (if possible) a baseline for a GeoNode openapi.json definition in the codebase.
  • The specification used for OpenAPI is based on OAS2 that is not maintained anymore and doesn't have some nice features available in OAS3. Also, the new generation of OGC APIs doesn't support OAS2. Is it possible to switch from OAS2 to OAS3? This would allow integrating pygeoapi eventually that supports only OAS3

I'd like to discuss the topic during the summit because I think it is strategic for the evolution of GeoNode in the long run.

@tomkralidis
Copy link
Member

Valuable and timely discussion here for the Summit next week. Comments based on pygeoapi viewpoint.

  • definitely agree with OAS3, which is what we are implementing in support of the OGC API standards
  • pygeoapi is built as a Python API which is capable of handling 1..n web frameworks (we support Flask and Django out of the box). For example, see our Flask support which is a very simple web routing layer atop our pygeoapi.api machinery. In similar "multiple API" projects that I am working on, we simply blueprint pygeoapi to, say /oapi and delegate accordingly. Here in GeoNode, perhaps /api/ogc could be a suitable option
  • pygeoapi builds the OAS3 document "by hand", i.e. (Python REST framework agnostic), as a build/deployment step. If we want both GeoNode and pygeoapi routes in the same OAS3 document, then we'll need a merge of the two of sorts when building/deploying
  • what are the auth considerations (pygeoapi does not do auth yet and would likely delegate to the downstream application)?

Having said this, whichever way the evolving GeoNode API is developed, it should be easy enough to build routes on top of pygeoapi for anything OGC API.

@t-book
Copy link
Contributor

t-book commented Dec 2, 2020

Thanks @francbartoli @tomkralidis

I like the general idea to get rid of the old API system.

From my point of view we shouldn't just disable old api versions for the sake of backward compatibility (but to offer /v1 /v2 /v3 depending on how the api develops.)
Furthermore I do not see the present proposal as a complete redesign but as a technical improvement of version 1. But of course this is open to discussion. I welcome the replacement of Tastypie with Django Rest Framework as it is lightweight, has a great documentation and is productively proven but I understand the question if pygeoapi can be an alternative.

Valuable and timely discussion here for the Summit next week. Comments based on pygeoapi viewpoint.

👍

I would like to know more about how far the development has progressed and what are the possibility with the auth layer.

Nobody would prevent us from considering v2 as a technically improved version of v1 (tastypie -> DRF) and offering a later v3 which is more OpenAPI centric and based on geopyapi.

@kalxas
Copy link
Member

kalxas commented Dec 2, 2020

First of all, thanks @afabiani for the proof of concept branch! It looks really good.
I also agree with @francbartoli and @tomkralidis that the new API should be based on OAS3 if possible.
@gannebamm yes, we should discuss how to interact with pygeoapi during the summit next week.

On the catalogue perspective I see some possible solution keeping the data model while working with the new OGC APIs at the same time: For resource base, we can keep/extend the model to keep compatibility with the CSW endpoint (which should not go away in my opinion). We can then re-use the resource-base model as a pygeoapi backend to support OGC-API Records, in a similar way we do with pycsw. For the rest of OGC APIs I echo @tomkralidis opinion.

@afabiani
Copy link
Member Author

afabiani commented Dec 2, 2020 via email

@afabiani
Copy link
Member Author

afabiani commented Dec 2, 2020 via email

@giohappy
Copy link
Contributor

giohappy commented Dec 2, 2020

As already said by @afabiani this is neither a proposal for the API of GeoNode 4, neither for a GeoNode "core". We wanted to make a step beyond the current one, experiment with new use cases as required by our clients which more and more ask for an extende REST interface (including for permissions, users management, etc.). Of course this API reflects somehow the underlying model (including the parts of it that we don't like much).

I don't see any conflict with pygeoapi here. Nothing prevents, now and in the future, haning both the interfaces. We already have pycsw for CSW, we can havd pygeoapi for OGC API. What are your concerns here @francbartoli?

@gannebamm
Copy link
Contributor

I see that the proposal does not interfere with other APIs and therefore voted my +1. Nonetheless we should give developers and users a clear way to interact with GeoNode API-wise. Having many different APIs alongside could cause confusion and convolute the codebase. This is something we should keep in mind.

@giohappy
Copy link
Contributor

giohappy commented Dec 3, 2020

I totally agree @gannebamm. Sometime users ask questions or requirements about the GeoNode "API", but then I discover they were talking about the OGC APis. Probably a dedciated section inside the docs might help to clarify the different exposed APIs.

afabiani pushed a commit that referenced this issue Dec 4, 2020
* [WIP] GeoNode API v2 - Prototype

* [ref. #6388] Error when installing core GeoNode into virtual environment

* - GeoNode REST APIs v2 - Swagger schema

* [ref. #6388] Error when installing core GeoNode into virtual environment

* - GeoNode REST API v2 : Map and MapLayers endpoints

* - GeoNode REST API v2 : Adding workflow ResourceBase metadata fields

* [Dependencies] Removing conflicts

* - Minor fixes and improvements

* - Improving ResourceBase REST api permissions

* - Improve Dynamic REST Sorting/Filtering

* - Expose more metadata fields

* - Expose more polymorphic_ctype_id field for filtering

* - Improve Metadata Fields serialization in order to make it searchable/filterable

* - Updating style sheets

* - Introducing "DynamicSearchFilter" exposing search_fields

* Merge branch 'master' of https://github.com/GeoNode/geonode into rest_api_v2_proof_of_concept

# Please enter a commit message to explain why this merge is necessary,
# especially if it merges an updated upstream into a topic branch.
#
# Lines starting with '#' will be ignored, and an empty message aborts
# the commit.

* - Allow gs proxy to parse workspace prefix dynamically

* - Downgrade pyproj to 2.6.1

* Revert " - Downgrade pyproj to 2.6.1"

This reverts commit a702740.

* - Improve Dockerfile

* [Issue #6414] Use Django storage API in delete_orphaned_* functions

* [Issue #6414] Use Django storage API in geonode.qgis_server.tests.test_views

* [Issue #6414] Use Django storage API when generating document thumbnails

* [Issue #6414] Thumbnail generation fix for local storage

* Add thumbnail convenience functions

* Cleanup Django storage API changes

* [Hardening] Minor improvements and error checks

* [Minor] Headers and formatting files

* - Adding "OAuth2Authentication" to the default REST APIs auth_classes

* - Fix "get_perms" api issue with non existant keys

* - Fix "get_perms" api issue with non existant keys

* [APIs] superusers have always perms to change resources

* [Dependencies] bump djangorestframework to >=3.1.0,<3.12.0

* - Improve IsOwnerOrReadOnly REST permission class

* - Improve IsOwnerOrReadOnly REST permission class

* - Improve IsOwnerOrReadOnly REST permission class

* - Fix Recenet Activity List for Documents

* [Hardening] - Recenet Activity List for Documents error when actor is None

* [Frontend] Monitoring: Bump "node-sass" to version 4.14.1

* [Frontend] Bump jquery to version 3.5.1

* [Fixes: #6519] Bump jquery to 3.5.1 (#6526)

(cherry picked from commit e532813)

# Conflicts:
#	geonode/static/lib/css/assets.min.css
#	geonode/static/lib/css/bootstrap-select.css
#	geonode/static/lib/css/bootstrap-table.css
#	geonode/static/lib/js/assets.min.js
#	geonode/static/lib/js/bootstrap-select.js
#	geonode/static/lib/js/bootstrap-table.js
#	geonode/static/lib/js/leaflet-plugins.min.js
#	geonode/static/lib/js/leaflet.js
#	geonode/static/lib/js/moment-timezone-with-data.js
#	geonode/static/lib/js/underscore.js

* Merge branch 'master' of https://github.com/GeoNode/geonode into rest_api_v2_proof_of_concept

# Please enter a commit message to explain why this merge is necessary,
# especially if it merges an updated upstream into a topic branch.
#
# Lines starting with '#' will be ignored, and an empty message aborts
# the commit.

* [Hardening] Re-create the map thumbnail only if it is missing

* Fixes error with GDAL 3.0.4 due to a breaking change on GDAL (https://code.djangoproject.com/ticket/30645)

* Fixes error with GDAL 3.0.4 due to a breaking change on GDAL (https://code.djangoproject.com/ticket/30645)

* - Introducing REST APIs for "Docuemnts" resources too

* - Bump django_mapstore_adapter to 2.0.6 / Bump django-geonode-mapstore-client to 2.0.9

* - Travis and LGTM fixes

* - Swagger OAS3

* - Adding REST API v2 Test Suite

* - Implementing API v2 "extent" filter

* - Travis and LGTM fixes

Co-authored-by: Matthew Northcott <matthewnorthcott@catalyst.net.nz>
Co-authored-by: Toni <toni.schoenbuchner@csgis.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
gnip A GeoNodeImprovementProcess Issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants