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

Introduction of WPS 2.0.0 templates and tests #345

Merged
merged 1 commit into from Jun 29, 2018

Conversation

@jachym
Member

jachym commented Jun 9, 2018

  • Rewriting outputs using Jinja2 templates
  • First version of 2.0.0 capabilities template

Overview

This patch deletes lxml-based output generation and introduces jinja2 templates, which can be used for more versios of the WPS standard.

It also adds new json() method where applicatble, to get JSON representation of each object easily.

related issue #337

Contribution Agreement

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

  • I have already previously agreed to the PyWPS Contributions and Licensing Guidelines

@jachym jachym added the enhancement label Jun 9, 2018

@jachym jachym added this to the 4.2.0 milestone Jun 9, 2018

@jachym jachym self-assigned this Jun 9, 2018

@jachym

This comment has been minimized.

Member

jachym commented Jun 9, 2018

  • get rid of xml generating methods for inputs and outputs
  • flake8
@coveralls

This comment has been minimized.

coveralls commented Jun 10, 2018

Coverage Status

Coverage increased (+2.0%) to 74.071% when pulling 0df6479 on jachym:templates into cde7d3d on geopython:master.

@coveralls

This comment has been minimized.

coveralls commented Jun 10, 2018

Coverage Status

Coverage increased (+1.03%) to 73.134% when pulling 0f7122d on jachym:templates into cde7d3d on geopython:master.

@jachym jachym changed the title from * Introduction of WPS 2.0.0 templates and tests to Introduction of WPS 2.0.0 templates and tests Jun 10, 2018

@tomkralidis

This comment has been minimized.

Member

tomkralidis commented Jun 10, 2018

Nice work! Comments:

  • what's the value proposition for template-based output vs. lxml proper? We took the lxml approach in pycsw years ago, and I'd be interested in hearing the pros/cons. I'd like us to move to jinja2 templates as well so hearing the benefits would be helpful
  • jinja templates typically have .j2 file extensions, you may want to reflect this in your template filenames
class BasicLiteral:
"""Basic literal Input/Output class
"""
def __init__(self, data_type="integer", uoms=None):
def __init__(self, data_type="integer", uoms=[UOM("metre")]):

This comment has been minimized.

@huard

huard Jun 11, 2018

Contributor

What's the rationale for setting meter as the default unit ? I think no unit would be preferable.

This comment has been minimized.

@jachym

jachym Jun 15, 2018

Member

My bad, there is no reason for default (mandatory) uom. Removed

@jachym

This comment has been minimized.

Member

jachym commented Jun 11, 2018

@tomkralidis

  1. IMHO there is big simplification of the code - I just make JSON structures, rest is in the template (No nodes generation
  2. step to MVC - view are the templates, it's much easier to add new versions now (see 2.0.0 capabilities vs. 1.0.0 capabilities) - yet the JSON is still the same.

I was unable to figure out, how to easy add support for WPS 2.0.0 :-/ This way, it works.

@tomkralidis

This comment has been minimized.

Member

tomkralidis commented Jun 11, 2018

Thanks @jachym makes sense. Any idea on performance comparison? This would be very interesting to see.

@jachym

This comment has been minimized.

Member

jachym commented Jun 17, 2018

DescribeProcess

<ows:DataType ows:reference="urn:ogc:def:dataType:OGC:1.1:float">float</ows:DataType>

or

<ows:DataType ows:reference="http://www.w3.org/TR/xmlschema-2/#float">float</ows:DataType>

what is correct?

@jachym

This comment has been minimized.

Member

jachym commented Jun 17, 2018

@tomkralidis Using flask server on local host

time for i in `seq 5000`; do \
      wget -q -O /dev/null "http://localhost:5000/wps?service=wps&version=1.0.0&request=describeprocess&identifier=all"; 
done

master

real	6m40,772s
user	0m10,699s
sys	0m14,827s

templates

real	10m45,675s
user	0m9,858s
sys	0m13,385s
@jachym

This comment has been minimized.

Member

jachym commented Jun 17, 2018

Examples are using https://www.w3.org/TR/xmlschema-2/#float, done

@tomkralidis

This comment has been minimized.

Member

tomkralidis commented Jun 17, 2018

So based on the results above, using jinja2 templating (vs. lxml output) takes (in real time) almost twice as much time to complete?

@jachym

This comment has been minimized.

Member

jachym commented Jun 18, 2018

@tomkralidis about 60% yes

in reality it is cca 0.1254s/request vs. 0.0774s/request

But note: it's just describe process (all processes), which still can be cached not execute with large raster data. And note also, that the execute calculation time is the one, which takes most. I'm personally fine (not happy) with the result.

@tomkralidis

This comment has been minimized.

Member

tomkralidis commented Jun 18, 2018

@jachym thanks. Fair enough. For the record I'm not opposed to the change, but interested in the differences. Note that it would be interesting to test once #346 is addressed.

@cehbrecht cehbrecht self-requested a review Jun 25, 2018

@cehbrecht

This comment has been minimized.

Collaborator

cehbrecht commented Jun 25, 2018

I have made a quick test using this PR in emu using Python 3.6. The getcapabilities request let to the following exception:

127.0.0.1 - - [25/Jun/2018 21:51:07] "GET /wps?service=WPS&request=GetCapabilities&version=1.0.0 HTTP/1.1" 500 -                    
Error on request:
Traceback (most recent call last):
  File "/Users/pingu/.conda/envs/emu/lib/python3.6/site-packages/werkzeug/serving.py", line 270, in run_wsgi                        
    execute(self.server.app)
  File "/Users/pingu/.conda/envs/emu/lib/python3.6/site-packages/werkzeug/serving.py", line 258, in execute                         
    application_iter = app(environ, start_response)
  File "/Users/pingu/.conda/envs/emu/lib/python3.6/site-packages/werkzeug/wsgi.py", line 766, in __call__                           
    return self.app(environ, start_response)
  File "/Users/pingu/.conda/envs/emu/lib/python3.6/site-packages/werkzeug/wrappers.py", line 311, in application                    
    return resp(*args[-2:])
  File "/Users/pingu/.conda/envs/emu/lib/python3.6/site-packages/werkzeug/wrappers.py", line 308, in application                    
    resp = f(*args[:-2] + (request,))
  File "/Users/pingu/tmp/PyWPS/pywps/response/capabilities.py", line 72, in __call__                                                
    doc = self.get_response_doc()
  File "/Users/pingu/tmp/PyWPS/pywps/response/__init__.py", line 75, in get_response_doc                                            
    raise e
  File "/Users/pingu/tmp/PyWPS/pywps/response/__init__.py", line 68, in get_response_doc                                            
    self.doc = self._construct_doc()
  File "/Users/pingu/tmp/PyWPS/pywps/response/capabilities.py", line 66, in _construct_doc                                          
    doc = template.render(**self.json)
  File "/Users/pingu/tmp/PyWPS/pywps/response/capabilities.py", line 23, in json                                                    
    processes = [p.json for p in self.processes.values()]
  File "/Users/pingu/tmp/PyWPS/pywps/response/capabilities.py", line 23, in <listcomp>                                              
    processes = [p.json for p in self.processes.values()]
  File "/Users/pingu/tmp/PyWPS/pywps/app/Process.py", line 90, in json                                                              
    'outputs': [o.json for o in self.outputs],
  File "/Users/pingu/tmp/PyWPS/pywps/app/Process.py", line 90, in <listcomp>                                                        
    'outputs': [o.json for o in self.outputs],
  File "/Users/pingu/tmp/PyWPS/pywps/inout/outputs.py", line 88, in json                                                            
    data = self._json_reference(data)
  File "/Users/pingu/tmp/PyWPS/pywps/inout/outputs.py", line 101, in _json_reference                                                
    data["href"] = self.get_url()
  File "/Users/pingu/tmp/PyWPS/pywps/inout/basic.py", line 711, in get_url                                                          
    (outtype, storage, url) = self.storage.store(self)
  File "/Users/pingu/tmp/PyWPS/pywps/inout/storage.py", line 96, in store                                                           
    file_block_size = os.stat(file_name).st_blksize
TypeError: stat: path should be string, bytes, os.PathLike or integer, not NoneType   


@cehbrecht

This comment has been minimized.

Collaborator

cehbrecht commented Jun 25, 2018

... using the PR with python 2.7 and emu I get the following exception for getcapabilities:

* Running on http://127.0.0.1:5000/ (Press CTRL+C to quit)
127.0.0.1 - - [25/Jun/2018 22:04:41] "GET /wps?service=WPS&request=GetCapabilities&version=1.0.0 HTTP/1.1" 500 -                    
Error on request:
Traceback (most recent call last):
  File "/Users/pingu/.conda/envs/emu/lib/python2.7/site-packages/werkzeug/serving.py", line 270, in run_wsgi                        
    execute(self.server.app)
  File "/Users/pingu/.conda/envs/emu/lib/python2.7/site-packages/werkzeug/serving.py", line 258, in execute                         
    application_iter = app(environ, start_response)
  File "/Users/pingu/.conda/envs/emu/lib/python2.7/site-packages/werkzeug/wsgi.py", line 766, in __call__                           
    return self.app(environ, start_response)
  File "/Users/pingu/.conda/envs/emu/lib/python2.7/site-packages/werkzeug/wrappers.py", line 311, in application                    
    return resp(*args[-2:])
  File "/Users/pingu/.conda/envs/emu/lib/python2.7/site-packages/werkzeug/wrappers.py", line 308, in application                    
    resp = f(*args[:-2] + (request,))
  File "/Users/pingu/tmp/PyWPS/pywps/response/capabilities.py", line 72, in __call__                                                
    doc = self.get_response_doc()
  File "/Users/pingu/tmp/PyWPS/pywps/response/__init__.py", line 75, in get_response_doc                                            
    raise e
TypeError: coercing to Unicode: need string or buffer, NoneType found                                                               


@jachym

This comment has been minimized.

Member

jachym commented Jun 26, 2018

@jachym

This comment has been minimized.

Member

jachym commented Jun 26, 2018

@cehbrecht 've tryed to install emu on my local host (using make but failed sofar). Do I understand correctly, that if I use anaconda, it will download complete python and all deps, independent on my system environment? I would be hoping for some pip install equivalent (just asking).

@cehbrecht

This comment has been minimized.

Collaborator

cehbrecht commented Jun 26, 2018

@jachym it looks like the Makefile is broken. You need to run the install steps manually:
http://emu.readthedocs.io/en/latest/installation.html#install-from-github

Make sure nothing was removed by the failed Makefile: git status

By using conda you get an independent environment for the Emu installation.

There is another issue with Emu. It is using the eggshell conda package which has a dependency to pywps conda package. You need to disable the OutputFormats process which uses eggshell:
https://github.com/bird-house/emu/blob/f37b6a4a77ad00809a12d555929125ad312f90a7/emu/processes/__init__.py#L14

@jachym

This comment has been minimized.

Member

jachym commented Jun 26, 2018

@cehbrecht The problem is, that your wordcounter process has as_reference set by default

https://github.com/bird-house/emu/blob/master/emu/processes/wps_wordcounter.py#L27

Then PyWPS tryes to construct output JSON element

https://github.com/jachym/PyWPS/blob/templates/pywps/inout/outputs.py#L87

And comes to the point, where the data should be part of the JSON structure (because JSON does not care, if it's constructed for GetCapabilities resepons or Execute response).

https://github.com/jachym/PyWPS/blob/templates/pywps/inout/basic.py#L711
and
https://github.com/jachym/PyWPS/blob/0f7122d63ad92e9e5de10bff990d1237f30b9212/pywps/inout/storage.py#L96

and there it failed.

I've just committed fix for this, which checks, if self.file exists. GetCapabilities seem to be working again and emu make test (after some tweaking with the output_formats process) passed too.

https://github.com/jachym/PyWPS/blob/templates/pywps/inout/outputs.py#L89

Thanks for testing - I hope, it works for you now as well?

Introduction of WPS 2.0.0 templates and tests
Rewriting outputs using Jinja2 templates
Introduction WPS 2.0.0 capabilities response
@cehbrecht

thanks for checking Emu issues. Sounds good :)

@jachym jachym merged commit 86a6a80 into geopython:master Jun 29, 2018

1 of 2 checks passed

coverage/coveralls Coverage increased (+1.02%) to 73.127%
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@jachym

This comment has been minimized.

Member

jachym commented Jun 29, 2018

And done, thank you all

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment