zopefoundation / transaction

Generic transaction implementation for Python.
Other
70 stars 31 forks source link

New transaction API for storing data on behalf of objects #15

Closed jimfulton closed 8 years ago

jimfulton commented 8 years ago

For objects such as data managers or their subobjects that work with multiple transactions, it's convenient to store transaction-specific data on the transaction itself. The transaction knows nothing about the data, but simply holds it on behalf of the object.

Discussion: https://groups.google.com/forum/#!topic/python-transaction/oUzj3uIHBgA

jamadden commented 8 years ago

Two thoughts:

First, if the implementation is going to be written in terms of id(key), then I really think that needs to be spelled out in the interface documentation. Otherwise, people are liable to write things like txn.set_data(self.__class__.__name__ + '.mykey'), which isn't guaranteed to have the same ID---once this API exists, it's likely to get used much more broadly then just in a ZODB Storage object. Even if they don't make that particular mistake, it seems like a good idea to spell out why the design is this way.

Second and much more minor, on PyPy in certain cases, calling id a lot "can lead to performance problem." (Also, some objects that have the same id under CPython as an implementation quirk are not guaranteed to by the language, and don't under PyPy, and vice-versa.) This is somewhat less a problem if this usage is well documented. But maybe we should allow for a key function that allows the user to determine how to hash the argument?

   def data(self, ob, key=id):
      return self._data[key(ob)]
jimfulton commented 8 years ago

I expanded the documentation in the interface.

WRT PyPY, what's "a lot"?

For my use case, data will be called for each transaction operation, so store, tpc_begin/vote/finish etc.

I'd rather not complicate this with something like a key function. (I'll reconsider if you you feel strongly about it though.)

jamadden commented 8 years ago

For PyPy, I don't know what constitutes "a lot", but as I understand it, the situations that can lead to that being a problem are not the default on most platforms. It was just a point to consider.

I'm fine with the doc updates, I think they cover my concerns, thanks. The interface can always be expanded later if necessary.

So, LGTM.