Changing base class for `Shared.DC.ZRDB.Aqueduct.Args` breaks existing ZSQLMethods

[dataflake]

@hannosch In commit f328208b2415e24026173435c5224abe20da966e, which was meant to be for PEP-8 compliance, you also changed the base class for Shared.DC.ZRDB.Aqueduct.Args to object where it did not have one before. This causes all ZSQL Methods created under Zope 2.13 and earlier to break:

2019-03-29 09:45:26 ERROR ZODB.Connection Couldn't load state for Products.ZSQLMethods.SQL.SQL 0x0358ec
Traceback (most recent call last):
  File "/opt/zope/.eggs/ZODB-5.5.1-py2.7.egg/ZODB/Connection.py", line 795, in setstate
    self._reader.setGhostState(obj, p)
  File "/opt/zope/.eggs/ZODB-5.5.1-py2.7.egg/ZODB/serialize.py", line 633, in setGhostState
    state = self.getState(pickle)
  File "/opt/zope/.eggs/ZODB-5.5.1-py2.7.egg/ZODB/serialize.py", line 626, in getState
    return unpickler.load()
TypeError: ('__init__() takes exactly 3 arguments (1 given)', <class 'Shared.DC.ZRDB.Aqueduct.Args'>, ())

Question: Was there a specific reason for this change? If not, I'd like to revert it unless you have a better suggestion. This is a blocker for Zope 4.

[hannosch]

Adding object to the base class was an intentional change, not merely a PEP8 thing.

The idea was to make the class into an explicit new-style class, so it would behave the same under Python 2 and 3 and avoid a migration during the 2to3 switch.

As for the traceback, I'm a bit surprised __init__ is called during unpickling at all. IIRC that shouldn't happen. I'd look at https://docs.python.org/2/library/pickle.html#pickling-and-unpickling-normal-class-instances and see if implementing __setstate__ with some migration logic helps.

The code and tests in DateTime might serve as some inspiration (https://github.com/zopefoundation/DateTime/blob/master/src/DateTime/DateTime.py#L456, https://github.com/zopefoundation/DateTime/blob/master/src/DateTime/tests/test_datetime.py#L255). The basic idea is to get the pickle dump in protocol version 1 (as used by the ZODB) representing an Args old-style class. Than do a pickle load in a Python environment, where the class is a new-style class. Put a pdb into the __setstate__ method and see what happens.

[dataflake]

So with your hints and the pickle documentation I have started debugging. I can "simulate" the issue by using the export/import dialog to import a ZSQL method exported from the old environment. Here's what I found:

With the current code, __init__ is called with an empty tuple as single argument, which is causing the error.

But what's confusing is that none of the other methods from the pickle documentation that supposedly get called during unpickling are called, especially not __setstate__. I have tried all combinations of adding __setstate__, __getinitargs__, __getnewargs__ etc but none of them get called at all, only __init__.

The only fix I could apply would be to have an __init__ that basically doesn't cares what's passed in, which is the case for your example, the DateTime class, and then try to fish potentially valid input out of *args. That seems such a dirty solution. What do you think?