geopython / pywps

PyWPS is an implementation of the Web Processing Service standard from the Open Geospatial Consortium. PyWPS is written in Python.
https://pywps.org
MIT License
177 stars 117 forks source link

pywps constructor broken (#39) #40

Closed landam closed 8 years ago

landam commented 9 years ago

It seems to me that 52379c5998da completely breaks pywps. It produces

Traceback (most recent call last):
  File "/home/landamar/src/PyWPS/wps.py", line 86, in <module>
    wps = pywps.Pywps(method)
  File "/home/landamar/src/PyWPS/pywps/__init__.py", line 179, in __init__
    config.loadConfiguration(configFiles, environ)
  File "/home/landamar/src/PyWPS/pywps/config.py", line 90, in loadConfiguration
    _overrideConfigFromConfigDict({k.split("_",2)[-1]:v for k,v in environ.iteritems() if k.startswith('PYWPS_CONFIG_')})
AttributeError: 'str' object has no attribute 'iteritems'
jachym commented 9 years ago

@deduikertjes could you have a look at it or will I remove the patch? sorry, apparently I did not test enough

deduikertjes commented 9 years ago

Hmm, without any info about config or setup this one is very difficult to debug.

But ... the first comment in my pull request states:

"give an entry for the request environment Maybe this breaks non-wsgi applications. Maybe something along the lines of: def init(self, environ = {"REQUEST_METHOD":"METHOD_GET"}, configFiles=None): is better."

I expect exactly that is happening and the user uses a cgi application.

landam commented 9 years ago

Right, it's a CGI application, in my case environ is GET (string) and not a dictionary.

landam commented 9 years ago

Your suggestion

def init(self, environ = {"REQUEST_METHOD":"METHOD_GET"}, configFiles=None):

will not work because environ is defined as a string before calling constructor (1)

(1) https://github.com/geopython/PyWPS/blob/master/wps.py#L86

deduikertjes commented 9 years ago

Change line 86 in wps = pywps.Pywps({"REQUEST_METHOD":"METHOD_GET"})?

ruz76 commented 8 years ago

My colleague wants to use pyWPS and he has problems, so I have tried it and got this error. I have changed line 86 to wps = pywps.Pywps({"REQUEST_METHOD":"METHOD_GET"}) and it worked partly for me. Now the error is on:

Traceback (most recent call last): File "/usr/local/bin/pywps.py", line 88, in if wps.parseRequest(inputQuery): File "/usr/local/pywps/pywps/init.py", line 213, in parseRequest self.inputs = self.parser.parse(queryStringObject) File "/usr/local/pywps/pywps/Parser/Post.py", line 94, in parse inputXml = file.read(maxFileSize) AttributeError: 'str' object has no attribute 'read'

deduikertjes commented 8 years ago

As stated in the comments in my pull request, I did not look at implications for CGI. As I have no CGI environment at my disposal I really cannot look into it.

jachym commented 8 years ago

Sorry, I'm reverting this change, I did not test it enough - it breaks the CGI and command line interface (for testing purposes a must) completely.

deduikertjes commented 8 years ago

Jachym,

I understand. Actually I expected to discuss the pull request before it was pulled in.

To prevent any more troubles I'd like to discuss the following BEFORE I make a pull request:

As I use uwsgi for wsgi deployment and pywps is not doing async requests in this setup, I've bitten the bullet and switched over to a CGI setup.

In this setup I've created the comparable functionality (the config is adaptable from keys in os.environ).

This is actually a very unobtrusive change requiring only this line (and the accompanying function) in config.py:

_overrideConfigFromConfigDict({k.split("_",2)[-1]:v for k,v in os.environ.iteritems() if k.startswith('PYWPS_CONFIG_')})

With the function being:

def _overrideConfigFromConfigDict(env_config):
    # In linux we can have camel case env_vars. In windows we'll run into some trouble.
    # for now, for windows we'll force section and option to lowercase
    for k,v in env_config.iteritems():
        section, option = k.split("_",1)
        try:
            if sys.platform == 'win32':
                config.set(section.lower(), option.lower(), v)
            else:
                config.set(section, option, v)
        except:
            pass    # fail silent logging is not ready yet

In my tests everything is working fine (even async requests). I actually cannot see how this setup would break things.

So, shall I create a pull request for this unobtrusive change?

MArco

jachym commented 8 years ago

Hi,

yes, it was my fault, I should've checked more detaily. As everybody, I'm very thankful for any pullrequest, which is supposed to solve one's problem.

ok, proposed function seems quite ok to me, please make the pullreques and I'll check, whether it works for usually use cases.

Thanks

tomkralidis commented 8 years ago

Any update on the status of this issue?