Cornices / cornice

Build Web Services with Pyramid.
https://cornice.readthedocs.io
Other
384 stars 149 forks source link

cornice-0.13 break colander schema's required field. #117

Closed shimizukawa closed 11 years ago

shimizukawa commented 11 years ago

https://github.com/mozilla-services/cornice/commit/461ed4b61f76735adaed278618beee842f6ad172#L2R138

I think this change break colander schema object that contain some marker object (colander.required and colander._marker). Because copy.deepcopy() copy marker object too, marker object's address will be changed.

almet commented 11 years ago

thanks for reporting this.

The solution I see is to do a deepcopy and then reinject the previous schema object. Would this work for you? Do you have a unittest I can use to reproduce the bug?

shimizukawa commented 11 years ago

I tried to make unittest and result is:

For example with colander-0.9.9:

>>> import colander
>>> import copy
>>> node = colander.SchemaNode(colander.String())
>>> node2 = copy.deepcopy(node)
>>> node2.required
False

python setup.py test on cornice-0.13 with colander-0.9.9 emits 3 errors.

Results:

  1. cornice <= 0.12 + colander (any versions): OK
  2. cornice >= 0.13 + colander <= 0.9.9: NG
  3. cornice >= 0.13 + colander >= 1.0a2: OK

I think (2) is a rare case, but it's a combination of the current stable versions.

My conclusion is: cornice-0.13 needs colander-1.0 or later. (early version is not acceptable).

almet commented 11 years ago

Thanks for this, that's effectively the case!