Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix migration documentation #661

Merged
merged 6 commits into from Aug 26, 2019
Merged

Fix migration documentation #661

merged 6 commits into from Aug 26, 2019

Conversation

icemac
Copy link
Member

@icemac icemac commented Jun 24, 2019

The main change is not to convert the ZODB in Python 2 as this is not the preferred way and the second attempt in Python 3 will fail because the ZODB is already converted.

The main change is not to convert the ZODB in Python 2 as this is not the preferred way and the second attempt in Python 3 will fail because the ZODB is already converted.
@icemac icemac requested a review from dataflake June 24, 2019 19:15
@icemac icemac self-assigned this Jun 24, 2019
@icemac icemac added this to In progress in Zope 4 bugfix via automation Jun 24, 2019
@icemac icemac added this to the 4.1.1 milestone Jun 24, 2019
Copy link
Member

@dataflake dataflake left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Omitting the ZODB migration step under Zope 4/Python 2 will not work for an older ZODB from Zope 2.13, which is my use case. Zope 4/Python 3 will refuse to open that ZODB because the magic marker is too old. The migration step under Zope 4/Python 2 will update the magic marker.

@dataflake
Copy link
Member

P.S.: And the migration under Python 3 does not fail, unless something in zodbupdate has changed radically in the last 2-3 weeks.

I think I ran through that procedure about 2 weeks ago and it worked as I documented it with a ZODB from Zope 2.13.26.

@icemac
Copy link
Member Author

icemac commented Jun 26, 2019

@dataflake That's strange. The removed zodbupdate call already creates a ZODB which is Python 3 only and converted to Python 3. Because the conversion is done in Python 2 land there have to be conversion tables for str objects containing the encoding.

Calling it again from Python 3 should lead to the error that the magic marker is not compatible as the ZODB is already converted. So this step is not necessary if the conversion is already done using Python 2. The documentation of zodbupdate suggests to prefer doing the conversion in Python 3 as it allows to use the --encoding parameter instead of the mapping tables in the code.

I personally converted a Data.fs from Python 3. It was created using Zope 2.13 and later used with Zope 4 on Python 2.

@dataflake
Copy link
Member

dataflake commented Jun 29, 2019

Have you run through that procedure recently?

I just tested it in a local sandbox with an actual production ZODB from Zope 2.13 in a sandbox using Zope 4/Python 3.7 as per your shortened instructions and the zodbupdate call fails. Looking at the line that raises the FileStorageFormatError error it reads the magic number and under Python 3 expects it to be FS30. That ZODB is FS21, though. The same code under Python 2 will expect FS21, which is why the two-step upgrade works.

I don't see how this can work at all when you run just a single zodbupdate step under Python 3.

$ bin/zodbupdate -n -v -f /opt/stage/5Ms_buildout/var/filestorage/Data.fs --convert-py3 --encoding utf-8 --encoding-fallback latin1
create storage /opt/stage/5Ms_buildout/var/filestorage/Data.fs
Traceback (most recent call last):
  File "bin/zodbupdate", line 129, in <module>
    sys.exit(zodbupdate.main.main())
  File "/opt/zope/.eggs/zodbupdate-1.2-py3.7.egg/zodbupdate/main.py", line 197, in main
    storage = ZODB.FileStorage.FileStorage(args.file)
  File "/opt/zope/.eggs/ZODB-5.5.1-py3.7.egg/ZODB/FileStorage/FileStorage.py", line 306, in __init__
    read_only=read_only,
  File "/opt/zope/.eggs/ZODB-5.5.1-py3.7.egg/ZODB/FileStorage/FileStorage.py", line 1619, in read_index
    raise FileStorageFormatError(name)
ZODB.FileStorage.FileStorage.FileStorageFormatError: /opt/stage/5Ms_buildout/var/filestorage/Data.fs

@dataflake dataflake dismissed their stale review July 25, 2019 14:56

Uncovered the real issue

@dataflake dataflake self-requested a review July 25, 2019 14:56
@icemac icemac added the RELEASE BLOCK Issue blocking the release/milestone it is in label Aug 9, 2019
@dataflake
Copy link
Member

I have spent some time on this and tried to somehow monkey-patch the expected Data.fs magic number into the code at runtime if specific conditions are met (dry run, Python 3), but have not managed a way to do it.

So for the migration instructions, the only rough outline I can suggest is the following:

UNDER PYTHON 2:

  • fix your code
  • run zodbverify

UNDER PYTHON 3:

  • run zodbupdate (drop the dry-run option)
  • run zodbverify

@dataflake
Copy link
Member

P.S.: As I couldn't find a way to beat the ZODB code into submission by monkeypatching it during the zodbupdate run, I am guessing the only way to make it all work is to change the ZODB code itself. The way the magic number is determined, imported and used in the FileStorage code would need to change so it is possible to influence it during runtime.

@dataflake
Copy link
Member

@icemac I have updated your branch and removed the dry-run step on Python 3, with a note added why. I also added a step to remove the ZODB index file before running the conversion, I am seeing consistent tracebacks that don't affect the conversion, but they are annoying. This may be ready for merging now.

@dataflake dataflake self-requested a review August 23, 2019 15:29
Copy link
Member

@dataflake dataflake left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good to go I think

@icemac icemac merged commit 82672cb into master Aug 26, 2019
Zope 4 bugfix automation moved this from In progress to Done Aug 26, 2019
@icemac icemac deleted the icemac-patch-1 branch August 26, 2019 05:56
@icemac
Copy link
Member Author

icemac commented Aug 26, 2019

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug documentation RELEASE BLOCK Issue blocking the release/milestone it is in
Projects
Zope 4 bugfix
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants