Commit b4c22280 authored by Jérome Perrin's avatar Jérome Perrin

remove some more numpy security duplication

this is already done in ERP5
parent a5c70293
...@@ -40,9 +40,6 @@ allow_module('scipy') ...@@ -40,9 +40,6 @@ allow_module('scipy')
allow_module('wendelin.bigarray.array_zodb') allow_module('wendelin.bigarray.array_zodb')
allow_type(np.timedelta64)
allow_type(type(np.c_))
import sklearn.linear_model import sklearn.linear_model
allow_class(sklearn.linear_model.LinearRegression) allow_class(sklearn.linear_model.LinearRegression)
......
  • @levin.zimmermann do you see something wrong with also removing here ? It's in ERP5 in https://lab.nexedi.com/nexedi/erp5/blob/f39ad5dfbfc66fb02e31bd38fc64e439cb5463bb/product/ERP5Type/patches/Restricted.py#L461-462 . I guess we missed it during 3f887532 because it's in another place in the file

  • oh right, thanks Jèrome and sorry that I missed it. For me it looks ok to remove it here. When already talking about this: I wonder if there's any particular reason why we don't move all of numpy related restricted python code to ERP5/Restricted.py (I mean this part and this one)? (is it considered to be too specific? But also for instance this looks arbitrary, but I guess those are just the parts which were needed, and then they may be needed for others too).

  • I don't know why we have all the numpy security related code in products/ERP5Type/patches/Restricted.py . I was also thinking we could have numpy and pandas handled in ERP5 and wendelin.core, sklearn and scipy in wendelin ... or maybe just move everything in ERP5 ! and maybe the security definitions for wendelin.core could be directly in wendelin core directly.

    If we do this, then we no longer need this Wendelin product, this could make the slapos setup a bit easier.

    There might be a reason to still have both, but this might just be historical. I think at the beginning, numpy and pandas were only in wendelin and then it was decided to have the "scientific stack" directly in ERP5 software release, but I don't remember.

  • I think all pandas related stuff has already been moved to erp5. For the remaining numpy related bits, I can prepare an erp5+wendelin patch.

    maybe the security definitions for wendelin.core could be directly in wendelin core

    you mean in https://lab.nexedi.com/nexedi/wendelin.core? maybe I misunderstand something, but wouldn't it then depend on RestrictedPython?

    Edited by Levin Zimmermann
  • If we do this, then we no longer need this Wendelin product, this could make the slapos setup a bit easier.

    From historical point of view...

    I think core idea is drop all products from all Projects, and incorporate the needs into a "Generic" ERP5 core.

    Despite the rest dont use scientific code, the general goal is to build a global core (with slapos software release and minimal set of ERP5 Products).

    At some point in time, even wendelin and slapos master bt5s were considered to be merged into erp5, but the idea was dropped early on (or postponed at least) to allow project (with public/opensource status) teams work faster since since merges on ERP5 could cause some extra work on early stage of development.

  • @rafael , that's entirely true from historic point of view. But then at some point if we want ERP5 on ARM architecture then we need also scientific libraries (openblas, etc) too. Then ERP5's SR becomes quite specific and / or simple hard to build on low powered architectures due to this (and actually for "scientific" libraries which are NOT going to be used there, too).

    So personally I'm still (like older times) of NOT having it all in ERP5. Not a question or debate for now just FYI of the context ...

  • you mean in https://lab.nexedi.com/nexedi/wendelin.core? maybe I misunderstand something, but wouldn't it then depend on RestrictedPython?

    sorry @levin.zimmermann I forgot to reply here. That was a good question. In my understanding, we might not need a dependency. What we are doing in this patch is take "foreign" classes (foreign as "not our code") and patch RestrictedPython so that it understand these classes, so that we can use __setitem__, __setattr__, __delitem__ and __delattr__ in restricted environment. But for the case of wendelin ZBigArray, it's "our code", so we can probably define the restricted versions of these methods ( __guarded_setitem__ etc ). We can even implement security checks if needed ( for example we could refuse setattr on instance if the class already has the attribute ). Because wendelin.core is intended to be used with ZODB, it's not so crazy to make the classes also implement some methods used in Zope's security system. I don't think much code is needed, maybe just __guarded__setitem__ = __setitem__ like here). I did not actually try all this, maybe we need more than just the __guarded_* methods.

    This approach of monkey patching a local variable in a function might also break in the future, with upcoming versions of RestrictedPython, so it would be good if we could use a more future proof approach. This is not really critical because it works now.

  • Toggle commit list
  • mentioned in commit f8678468

    Toggle commit list
  • Thanks for your explanation @jerome! I understand this better now, this sounds very reasonable.

  • mentioned in commit Demonkey/wendelin@bd3bd70c

    Toggle commit list
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment