Expose extra_engine_options to the UI

[dwt]

BUG/PROBLEM REPORT (OR OTHER COMMON ISSUE)

Currently the code contains the attribute extra_engine_options that is neither visible directly in the UI nor settable in the UI.

What I did:

To set it we wrote a python script, got the wrapper via acquisition, and then set an option on it via the add_extra_engine_options() method.

What I expect to happen:

That is not very discoverable and not really nice. It would be much preferable to be able to write down such options in a property on the SA wrapper itself and also view what is currently set.

I am not sure what Zope supports here, but maybe the config dictionary can be written down as json in the config UI? That should be fairly safe.

What actually happened:

Trickery with an extra python script to set this option.

What version of Python and Zope/Addons I am using:

centos 7, python 3.6, latest zope and sqlalchemyda.

[dataflake]

Unrelated to this particular issue, I see you're using Python 3.6. You should be aware that due to a waitress bug that we cannot fix on the Zope side we recommend everyone move to at least Python 3.7: https://community.plone.org/t/zope-4-8-1-and-5-5-1-released/15113

As far as this issue is concerned, patches are always welcome.

[dwt]

I guess you mean move to python 3.7? Noted and thanks for the heads up.

[dataflake]

Yes, that was a typo

[fixader]

Not a straight forward answer to this one i think. Here is my one cent worth of input (definitely not worth two) I have had a few days to play around with this product. sorry for not copy pasting code, thats due to the server sitting in a windows hypervisor on a remote server, and copy pasting doesnt really work.

So i added an extra option to play with: user_engine_options =''. This just to change as little as possible. I then just made a try except in engine_options : try: if self.user_engine_options!='': engine options=eval(self.user_engine_options) ecept: pass

this allows me to fill out the property with something like: {'client_encoding':'utf8','executemany_mode':'values_only'}

These options here are supported by my db and dialect (postgresql+psycopg2) others will support other options. EDIT: if i for instance use a wrong value for "executemany_mode" for exampe "values only" (space not underscore) it raises an error sqlalchemy.exc.ArgumentError : "Invalid value for executemany_mode : 'values only'" as it should

This is an ugly kludge, but still better than using an extra script. if options are unsupported you can use the back button to change the properties back or to an empty one at least. And the crash happens when u save options.

IF databases and connectors all would support a nice list of options in a unified way this would be easy. They dont. So doing this in a way that verifies options in a way that gracefully exits is not easy

[dataflake]

Blindly using eval on user-provided input seems like a foot gun to me. This is not something I could accept.

[dwt]

@fixader I was thinking that json.parse(self.user_engine_options) should be good enough to get this options safely. Would you care to provide that as a patch? I have to say that my zope-foo is sadly not quite up to the task to provide this patch and tests for it correctly.

[fixader]

@dataflake I couldnt agree more that using eval is not a good solution. That is its own can of worms. But it was more a way for me to find out what would and what wouldnt work in an easy way. @dwt i dont feel up to posting patches for everyone. I implemented your suggestion of using json, so i wrapped that in a try/except and if the creation of a dictionary failed it reverted to empty. Much better, with at least some rudimentary valdidation. You can still screw things up tho. I posted u the resulting da.py privately to keybase. Feel free to try it out.