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

Issue 16 #51

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Issue 16 #51

wants to merge 3 commits into from

Conversation

jorgejesus
Copy link
Member

@jorgejesus jorgejesus commented Dec 30, 2018

Overview

Implements issue #16 (Implementaition of PYWPS_CFG and PYWPS_PROCESSES). The demo.py script will pick up the variables from system env and use it. Unitests was write and located in tests/test_variables.py

Related Issue / Discussion

@tomkralidis Is it necessary for the directory in PYWPS_PROCESSES to be checked for changes and automatically reloaded or is it enough to restart pywps4-flask with a new PYWPS_PROCESSES

There is also a #11 request that needs evaluation based on changed of #51

Additional Information

test_variables.py doesn't used a live server, and will just make some requests to wps() on demo.py and check that temporary PYWPS_PROCESSES and PYWPS_CFG were accepted

Travis was updated but requires a bit more attention

Contribution Agreement

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

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

@jorgejesus jorgejesus mentioned this pull request Dec 30, 2018
@tomkralidis
Copy link
Member

Thanks @jorgejesus. It's enough to restart pywps4-flask with a new PYWPS_PROCESSES.

@cehbrecht cehbrecht self-requested a review January 8, 2019 09:44
Copy link

@cehbrecht cehbrecht left a comment

Choose a reason for hiding this comment

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

Code changes look fine ... but I haven't tested.

@cehbrecht
Copy link

@jorgejesus The feature to load processes from a directory is useful in general and we might want to have it in pywps itself (again) ... could be added as an option. It would be kind of a plugin mechanism. A simple implementation to start with is fine.

Also we could move the usage of PYWPS_CFG environment variable to pywps.

@jorgejesus
Copy link
Member Author

@jorgejesus The feature to load processes from a directory is useful in general and we might want to have it in pywps itself (again) ... could be added as an option. It would be kind of a plugin mechanism. A simple implementation to start with is fine.

Also we could move the usage of PYWPS_CFG environment variable to pywps.

I can open a ticket on pywps for introducing a plugin structure for processes and reintegration of PYWPS_CFG

@cehbrecht
Copy link

@jachym @jorgejesus @tomkralidis Should we merge this PR? ... or take it as an example and implement it pywps geopython/pywps#118.

It has a new example process TotalLength ... but could be added in another PR.

@jachym
Copy link
Member

jachym commented Feb 16, 2020

this is an oldone - don't we have already code for automatic process import in the master branch ? processes configuration option, which is pointing to Python module?

@cehbrecht
Copy link

this is an oldone - don't we have already code for automatic process import in the master branch ? processes configuration option, which is pointing to Python module?

yes. We could extend it to load processes from a file path, like done in this PR.

@jorgejesus
Copy link
Member Author

@cehbrecht, @jachym is this PR still valid/important or we can trash it ?

@cehbrecht
Copy link

I think we don't want to merge it but keep it as reference for the implementation of the processes_path (geopython/pywps#118).

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.

4 participants