client: clean up import/export code

Export:
- Remove leftover warning about a bug that was fixed in
  commit e76af297
- In neomigrate script, open NEO storage read-only.
- IStorageIteration is already implemented.

Import:
- Review comments.
- In neomigrate script, warn that IStorageRestoreable is not implemented.
- Do not call 'close' method on source iterator. BaseStorage does not do it and
  this is not part of ZODB API. In the case of FileStorage, resource are freed
  automatically during garbage collection.

TODO

@@ -142,8 +142,8 @@ RC  - Review output of pylint (CODE)
     - Use generic bootstrap module (CODE)
     - If too many storage nodes are dead, the client should check the partition
       table hasn't changed by pinging the master and retry if necessary.
-    - Implement restore() ZODB API method to bypass consistency checks during
-      imports.
+    - Implement IStorageRestoreable (ZODB API) in order to preserve data
+      serials (i.e. undo information).
     - tpc_finish might raise while transaction got successfully committed.
       This can happen if it gets disconnected from primary master while waiting
       for AnswerFinishTransaction after primary received it and hence will

neo/client/Storage.py

@@ -32,13 +32,8 @@ class Storage(BaseStorage.BaseStorage,
 
     implements(
         ZODB.interfaces.IStorage,
-        # "restore" missing for the moment, but "store" implements this
-        # interface.
         # ZODB.interfaces.IStorageRestoreable,
-        # XXX: imperfect iterator implementation:
-        # - start & stop are not handled (raises if either is not None)
-        # - transaction isolation is not done
-        # ZODB.interfaces.IStorageIteration,
+        ZODB.interfaces.IStorageIteration,
         ZODB.interfaces.IStorageUndoable,
         ZODB.interfaces.IExternalGC,
         ZODB.interfaces.ReadVerifyingStorage,
@@ -178,8 +173,7 @@ class Storage(BaseStorage.BaseStorage,
 
     def copyTransactionsFrom(self, source, verbose=False):
         """ Zope compliant API """
-        return self.app.importFrom(source, None, None,
-                self.tryToResolveConflict)
+        return self.importFrom(source)
 
     def importFrom(self, source, start=None, stop=None, preindex=None):
         """ Allow import only a part of the source storage """

neo/client/app.py

@@ -919,23 +919,25 @@ class Application(object):
 
     def importFrom(self, source, start, stop, tryToResolveConflict,
             preindex=None):
+        # TODO: The main difference with BaseStorage implementation is that
+        #       preindex can't be filled with the result 'store' (tid only
+        #       known after 'tpc_finish'. This method could be dropped if we
+        #       implemented IStorageRestoreable (a wrapper around source would
+        #       still be required for partial import).
         if preindex is None:
             preindex = {}
-        transaction_iter = source.iterator(start, stop)
-        for transaction in transaction_iter:
+        for transaction in source.iterator(start, stop):
             tid = transaction.tid
             self.tpc_begin(transaction, tid, transaction.status)
             for r in transaction:
                 oid = r.oid
                 pre = preindex.get(oid)
-                # TODO: bypass conflict resolution, locks...
                 self.store(oid, pre, r.data, r.version, transaction)
                 preindex[oid] = tid
             conflicted = self.tpc_vote(transaction, tryToResolveConflict)
             assert not conflicted, conflicted
             real_tid = self.tpc_finish(transaction, tryToResolveConflict)
             assert real_tid == tid, (real_tid, tid)
-        transaction_iter.close()
 
     from .iterator import iterator
 

neo/scripts/neomigrate.py

@@ -44,18 +44,17 @@ def main(args=None):
 
     # open storages
     from ZODB.FileStorage import FileStorage
-    #from ZEO.ClientStorage import ClientStorage as ZEOStorage
     from neo.client.Storage import Storage as NEOStorage
     if os.path.exists(source):
+        print("NOTE: NEO does not implement IStorageRestoreable interface,"
+              " which means that undo information is not preserved: conflict"
+              " resolution could happen when undoing an old transaction.")
         src = FileStorage(file_name=source, read_only=True)
         dst = NEOStorage(master_nodes=destination, name=cluster,
                          verbose=options.verbose)
     else:
-        print("WARNING: due to a bug in FileStorage (at least up to ZODB trunk"
-              "@121629), output database may be corrupted if input database is"
-              " not packed.")
         src = NEOStorage(master_nodes=source, name=cluster,
-                         verbose=options.verbose)
+                         verbose=options.verbose, read_only=True)
         dst = FileStorage(file_name=destination)
 
     # do the job