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

Adds extensible possibilities to the API class to support dynamic resources loading and hot-reload logic mechanisms #1841

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Alex-NRCan
Copy link
Contributor

@Alex-NRCan Alex-NRCan commented Nov 5, 2024

Overview

This PR adds 2 methods and 2 overridable methods in the API class to allow for:

  • (1) loading resources from somewhere else than the yaml file and;
  • (2) hot-reload the resources dynamically, through multiple pygeoapi instances (e.g. in a load balanced environment) without having to restart any pygeoapi instances.

This PR also adds a decorative function to make sure the resources are always up to date with the possibly dynamic resources material.

With the overridable functions implementations, this PR doesn't alter the native way pygeoapi works.

Related Issue / discussion

Additional information

Dependency policy (RFC2)

  • I have ensured that this PR meets RFC2 requirements

Updates to public demo

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

@totycro
Copy link
Contributor

totycro commented Nov 6, 2024

Is last_loaded_resources meant to check file modified times? I was just wondering how to deal with a case where the list of resources would be loaded from a server, we wouldn't necessarily have a modified time. But then one could still reload the list every time (or use a response that's cached for some time).

It might be nice to have a tiny example implementation of this so it's easier to see how this would be used. Since tests are also missing right now, maybe a example could be part of a test?

@Alex-NRCan
Copy link
Contributor Author

Alex-NRCan commented Nov 6, 2024

Hey totycro,

Thank you for your comment.
last_loaded_resources is used to keep track of the last time the resources were loaded in the pygeoapi instance, indeed.

That way, in the custom implementation override (i.e in on_load_resources_check function), one can verify if that date is "too old" compared to whatever principle one has in place to determine that.

For example, in our implementation, we have all resources stored in a database (not the yaml file), because we have multiple pygeoapi instances running in parallel (load balanced) and we wanted them all to use the resources centralized in a database and those resources can change any time. When the resources change, we flag a date in the database about it. Therefore, in our on_load_resources_check implementation, we check if the running pygeoapi instance's last_loaded_resources date is older than the date in the database and when so, that particular pygeoapi instance reloads its resources automatically without rebooting (simply because we return true in our on_load_resources_check implementation.

That being said, one can have different ways of checking for a reload flag, it doesn't have to be a central database. That is why, in the implementation I'm suggesting, the on_load_resources_check simply returns a boolean. It's up to the developer to implement their custom check. Similarly, it's also up to the developer to implement on_load_resources which can be anything, not necessarily a central database either.

To your point also, yes, if one implements on_load_resources_check to always return True, then the resources will always be reloaded before responding to the request. Not very optimal performance wise, but it'd work.

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.

2 participants