Simplify the implementation of IOHandler

[gschwind]

Overview

The current implementation use a tricks that manipulate the mro. This trick do not have any obvious benefit and make the code much more complicated than necessary. This patch simplifier the implementation by replacing the tricky class change to a change in mamber's variable.

Related Issue / Discussion

Following discussion in #598, Here is my proposal of refactoring the IOHandler. It come in two flavor, one with is a basic one and a second using weakref. I thought of another way using only class object without instantiating them like the previous implementation but I don't like too much the idea.

The code pass all the test excluding two. the two failed test a due to the fact that the test use private API to trick the test checking, thus IMO this just the test that is invalid.

Best regards.

Additional Information

Contribution Agreement

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

• [x] 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

[huard]

I've tested this with emu + birdy and I'm getting 3 INTERNAL SERVER ERRORS.

https://github.com/bird-house/emu https://github.com/bird-house/birdy

To reproduce:

1. Install emu and launch it with emu start
2. Install birdy and run the test suite with pytest

[gschwind]

Hello huart,

I will have a look, best regards.

[coveralls]

Coverage remained the same at 0.0% when pulling babc8fcf686e2feeafa55e36753265dca9285a63 on gschwind:pywps-4.4-007 into 559f13b960b2ba17599818e05c047ce3f1b2e54d on geopython:pywps-4.4.

[gschwind]

Hello huard,

I found the guilty :)

Thank you for your feedbacks

Best regards

[cehbrecht]

@gschwind @huard merged . Thanks :)

[huard]

Hi @gschwind

I'm working on parallelism in PyWPS and I'm hitting an issue with the serialization of Process instances. The IO handlers include weakrefs that are not picklable, and so I'm not able to send processes to "workers". Could we replace those weakref by something else that would be pickable?

[gschwind]

Hello huard,

Why you do not use json serialization as it's done currently to store request in data base ?

Best regards.

[huard]

I tried to add __setstate__ and getstatemethod using thejsonproperty andfrom_json` method, but it broke pickling. Replacing the weakrefs by normal references solved this issue, but I wanted to know what were the downsides. I might be missing something obvious. I'll open a separate issue about PyWPS object serialization.

[gschwind]

I guess there is downside, I don't think I use weakref if there is no need of it. I have to check. The two main reason to use it. One is for breaking circular dependency with object references such as A->B->A that cause memory leak and require garbage collector to fix it. Second is to not take the ownership of the variableand being able to check if the owner destroyed it (released it).

I have to check. In other hand I would not use pickle to transfer python object from one process to another, but I do not know your use case.

Best regards.

[huard]

Ok thanks, see #658.

[gschwind]

Hello,

After a quick check, it is for breaking circular references. for instance within the code IOHandler._iohandler is a refecence to FileHandler or DataHandler and those object has FileHandler._ref and DataHandler._ref that is linked to there owner IOHandler. If I do not use weakref here I have a reference loop where IOHandler own FileHandler or DataHandler, and those FileHandler or DataHandler own their parent IOHandler.

If you want as dirty fix, you can use prepare_serialization that remove the weak ref with None and fix_deserialize that rebuild the weak_ref. like:

def prepare_serialization(self):
   self._iohandler._ref = None

def fix_deserialize(self):
    self._iohandler._ref = weakref.ref(self)

Best regards

[huard]

Many thanks for the explanation and suggestion.