perfact / zodbsync

Dump and restore objects between a ZODB and a serialized file system structure
GNU General Public License v2.0
12 stars 5 forks source link

Allow for a callable title on objects in ZODB. #108

Closed sallner closed 1 year ago

sallner commented 1 year ago

During the serialization of forms from Products.Formulator an error occurred as in some cases the title of an object was a callable.

viktordick commented 1 year ago

I did not know that objects with non-plain titles could exist. I guess if such objects need to be serialized and also played back into the ZODB, I would rather try to access the data that determines the output of the title function instead of calling it - if it is a function and could depend on different data behind the scenes, simply recording the result of the function will not give enough information to restore the object later on. That being said, recording such objects should of course not fail and at least give an unsupported dummy. But I think I would prefer to simply omit the key meta['title'] in that case rather than setting it to the result of the callable.

What do you think @sallner? Would it be sufficient for your use case if the resulting serialization simply had no title field?

On another note: We try to still support Python 2 with this package (still have some systems with Zope 2 in the field and zodbsync is a tool we use when upgrading old systems containing some custom code to a current code base). It seems that mock is not part of the standard library in Python 2 (at least on my archlinux, running the tests for Python 2 complained that mock could not be imported), so it should be added to .toxfiles/reqs-py2.txt.

EDIT: Just saw that you have already added the dependency. I guess my tox call used the cached environment and did not update to fetch this dependency.

sallner commented 1 year ago

I think that it would be an option to have the meta['title'] omitted. I can imagine, that a meta['custom_title'] in read() and write() could be an option for the current problem in my situation.

For the reference: The problem occurred while serializing Fields in Forms of Products.Formulator.

Would you like to implement a solution for the omission of meta['title']?

viktordick commented 1 year ago

If following my suggestion, you could simply change the code snippet to

    title = getattr(obj, 'title', None)
    # see comment in helpers.py:str_repr for why we convert to string
    if isinstance(title, (six.binary_type, six.text_type)):
        meta['title'] = to_string(title)

The tests also needs to be adjusted, of course.

I am not sure what you are trying to achieve. zodbsync is designed to serialize objects as fully as possible to allow a ZODB that contains page templates, python scripts, document templates and similar object types to be stored on the file system for git workflows and transfering between systems. In particular, as far as I understand, our use case is somewhat special in that we (at PerFact) use the ZODB primarily as code storage and not for holding transactional data (we use a PostgreSQL database for that).

This means that zodbsync has a specific list of object types that it supports and is also able to play back (see object_types.py). I have no problem in accepting additional supported types and there is probably only partial support for some of the types that we do support, because we might not use some feature (compare #87, which documents an incomplete support for access rules, but I never came around to fix this because we do not actually use them and there is no demand for it that I know of).

Currently, any object that we do now support is recorded with a dummy meta file that contains the name of the meta type and an unsupported key. It also until now always contained the title because I never encountered an object type that had no POD title until now, but if this assumption was wrong all this time, I have no problem with omitting the title.

I am a little bit opposed to simply evaluating the title if it is a callable. If you need full support for Fields from Formulator, you would need to add a corresponding handler in object_types.py, which parses obj.tales or whatever actually determines the output of obj.title() - that way a playback of the serialized object back into the ZODB could also be achieved. As of now, only a bare meta file with unsupported marker is created and my guess is that it does not make much of a difference for you if this file contains a title key or not.

My reason for favoring the omission of the title key altogether in such cases is historical. There have been multiple instances where we used a convenience function at first, but then switched to reading out the underlying data directly due to inconsistencies between playback and record or between what the watcher recorded without having the full acquisition path to the object available and what a record returned (for example, this or this).

sallner commented 1 year ago

I am happy with your proposed solution and will implement it in this PR and adapt the tests.

The general structure of my current project reflects yours in the sense that code is stored in the ZODB. The forms form the formulator are also treated as code in the sense, that they are archived and versioned with git.

I am aware of object_types.py and have in fact already created some classes for our use cases. I changed also all of the other fields in the __meta__ file so that we can dump and load them in completely. The title was just the only one I could not modify with the help of a ModObj subclass.

icemac commented 1 year ago

@viktordick I implemented your suggestions, could you please review?