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

Add Admin API #1137

Merged
merged 34 commits into from
Jan 3, 2024
Merged

Add Admin API #1137

merged 34 commits into from
Jan 3, 2024

Conversation

webb-ben
Copy link
Member

@webb-ben webb-ben commented Feb 10, 2023

Overview

This PR introduces an OGC API - Features - Part 4 inspired admin API around configuration of pygeoapi resource configurations. It has been implemented and tested for a couple of months as a part of the WMO wis2box OSS software developments efforts in wis2box-api.

@ksonda @tomkralidis Interested to hear what you think about the next steps

Tasks that I am aware still need to be completed:

  • Better unit & integration testing for admin API. Might make sense to move the admin api tests to their own CI job.
  • conservation of environment variables.
  • deploy the admin CI with a mechanism better than the building the docker example.
  • Implementation of corresponding routes with Starlette and Django
  • Remove vue from HTML templates
  • Create Admin API documentation

Related Issue / Discussion

#756, #728

Additional Information

Demo deployment exists in the docker examples folder. To Hotload the changes from the API to the configuration, requires changes like that made in the examples entrypoint script.

Contributions and Licensing

(as per https://github.com/geopython/pygeoapi/blob/master/CONTRIBUTING.md#contributions-and-licensing)

  • I'd like to contribute [feature X|bugfix Y|docs|something else] to pygeoapi. I confirm that my contributions to pygeoapi will be compatible with the pygeoapi license guidelines at the time of contribution.
  • I have already previously agreed to the pygeoapi Contributions and Licensing Guidelines

@webb-ben webb-ben requested a review from tomkralidis February 10, 2023 17:48
@webb-ben webb-ben marked this pull request as draft February 10, 2023 17:50
@tomkralidis tomkralidis self-assigned this Feb 10, 2023
@webb-ben
Copy link
Member Author

webb-ben commented Sep 21, 2023

Updated with some notes additional todos:

@webb-ben
Copy link
Member Author

HTML view:
image

Copy link
Contributor

@francbartoli francbartoli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @webb-ben, this is a great addition for administrators. However, I was wondering if this Admin API can be made optional for the implementers who want lean and clean OGC APIs without any extra stuff and so do not challenge with security requirements with the introduction of this.

@tomkralidis
Copy link
Member

It would be valuable to have this configurable in the config.

@webb-ben
Copy link
Member Author

webb-ben commented Sep 21, 2023

It would be valuable to have this configurable in the config.

@tomkralidis @francbartoli per our previous discussion on this in #756, it is a boolean in the server section:

https://github.com/geopython/pygeoapi/pull/1137/files#diff-631fe735514040ce51f87cfb0c8eeeb1b7895d42c9364cea597355062732c159R466-R467

Copy link
Contributor

@francbartoli francbartoli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@webb-ben are django_admin.py and starlette_admin.py files still missing?

@webb-ben webb-ben mentioned this pull request Sep 22, 2023
2 tasks
@webb-ben
Copy link
Member Author

@francbartoli The admin API has not been implemented for starlette or django - I am new to both of those frameworks.

@webb-ben webb-ben marked this pull request as ready for review September 26, 2023 21:03
@webb-ben webb-ben changed the title [WIP] Add Admin API Add Admin API Sep 26, 2023
Copy link
Contributor

@francbartoli francbartoli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What would be the benefit to introduce another json related dependency? (I'm asking because we discussed to eventually remove pydantic soon)
Also, since this admin functionality is designed to be optional I wouldn't keep it in the core dependencies. It's better to move it under a requirements-admin.txt file

@tomkralidis
Copy link
Member

@francbartoli the dependency is for JSON merge patching on PUT/PATCH transactions. Having said this, +1 to move to requirements-admin.txt.

- Create `admin.py` to serve as Admin API Core
- Create `flask_admin.py` to create flask blueprint for admin API
- Consolidate configuration getter
- Add Pathlib serializing
- Add docker example
- Amend admin example to allow writing to configuration. If FS is read only admin API does not work. Returns a 500 and logs `OSError: [Errno 30] Read-only file system: '/pygeoapi/local.config.yml' `
webb-ben and others added 17 commits January 2, 2024 20:33
Update admin entrypoint to align with upstream pygeoapi implementation
Co-Authored-By: Tom Kralidis <[email protected]>
Co-Authored-By: Tom Kralidis <[email protected]>
- Add put and patch for root configuration
- Add CI tests for PUT and PATCH of root
Replace tabs with spaces
Error from rebasing. Admin API tests are moved to their own job.
- Use debian supported packaging
- Use custom merge function
- Fold flask_admin.py into flask_app.py

Co-Authored-By: Tom Kralidis <[email protected]>
Move admin dependencies to requirements-admin.txt
Update expected test count for addt'l admin test data
@tomkralidis tomkralidis merged commit 8e122d1 into geopython:master Jan 3, 2024
3 checks passed
@tomkralidis tomkralidis mentioned this pull request Jan 3, 2024
@webb-ben webb-ben deleted the admin branch January 3, 2024 16:01
webb-ben added a commit to geopython/pygeoapi-examples that referenced this pull request Jan 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants