Opened 11 years ago
Closed 9 years ago
#10353 closed defect (fixed)
Remove zodb from sage
Reported by: | was | Owned by: | jason |
---|---|---|---|
Priority: | major | Milestone: | sage-5.7 |
Component: | misc | Keywords: | |
Cc: | kini | Merged in: | sage-5.7.beta1 |
Authors: | François Bissey | Reviewers: | Jeroen Demeyer |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | #12205, #13717, #13963 | Stopgaps: |
Description (last modified by )
zodb only use the pickle protocol version 1. It is also not used in sage apart from one package that has been migrated away from it in #12205.
There is a zodb mailing list discussion related to the pickle protocol here: http://www.mail-archive.com/zodb-dev@zope.org/msg04628.html
In particular, pickle protocol 1 is hardcoded. But SageObjects/Cython? objects must use protocol 2 in many cases:
sage: import cPickle sage: F = factor(12) sage: cPickle.dumps(F, protocol=0) TypeError: can't pickle SageObject objects sage: cPickle.dumps(F, protocol=1) TypeError: can't pickle SageObject objects sage: cPickle.dumps(F, protocol=2) '\x80\x02csage.structure.factorization\nFactorization\nq\x01)\x81q\x02}q\x03(U\x12_Factorization__crq\x04\x89U\x14_Factorization__unitq\x05csage.rings.integer\nmake_integer\nq\x06U\x011\x85Rq\x07U\x18_Factorization__universeq\x08csage.rings.integer_ring\nIntegerRing\nq\t)Rq\nU\x11_Factorization__xq\x0b]q\x0c(h\x06U\x012\x85Rq\rK\x02\x86q\x0eh\x06U\x013\x85Rq\x0fK\x01\x86q\x10eub.'
See also the related ticket #10352.
The suggested option for this ticket is to remove zodb from sage.
- Apply trac10353-db-removal.patch to the sage library
- Apply trac10353-sage_root.patch to the sage root repository
- Remove the zodb package.
Attachments (2)
Change History (30)
comment:1 Changed 10 years ago by
comment:2 Changed 10 years ago by
I added ZODB to Sage (long ago), and I am definitely +1 to removing it from Sage, if we can figure out how. I think it is a technology whose value (for Sage!) has come and gone. One can do better with SQLite these days.
comment:3 Changed 10 years ago by
OK it is really a pain on sage-on-gentoo side and will become worse in a few months (removal from the main tree, zope maintenance is apparently hard that it will be put in its own repo for people who are interested in it).
So I am putting my hand up to remove zodb from sage. Anything else apart from conway polynomial database needs to be cleaned and converted to sqlite?
comment:4 Changed 10 years ago by
- Cc kini added
comment:5 Changed 9 years ago by
It looks like stein-watkins-ecdb also use zodb.
comment:6 Changed 9 years ago by
Hum it looks like sage/databases/stein-watkins-ecdb.py imports sage.databases.db but it is not actually using any of it as far as I can see. So it will be a simple patch to get rid of db.py and compressed_storage.py.
comment:7 Changed 9 years ago by
If I'm not wrong, sagenb uses some Python packages which are currently included in zodb spkg (I think zope.interface or stg like that, see #10352).
comment:8 Changed 9 years ago by
If I remember the discussion correctly the dependency on zope.interface will be included in a future version of sagenb. My quick check on the current version of sagenb show no import from zope. It was suggested that it should be included in the sagenb bundle, I personally agree with the idea. In any case that would mean replacing the current zodb spkg with a new one cotaining only zope.interface if we want to keep it in main line.
comment:9 Changed 9 years ago by
I just attached a patch to remove zodb references from the sage library. I haven't done the patch for sage-root yet.
comment:10 Changed 9 years ago by
- Dependencies set to 12205
- Description modified (diff)
- Status changed from new to needs_review
- Summary changed from ZODB is basically useless in Sage, since it uses pickle protocol 1 to Remove zodb from sage
comment:11 Changed 9 years ago by
- Dependencies changed from 12205 to #12205
comment:12 Changed 9 years ago by
I believe the line in spkg/standard/deps
$(INST)/$(SQLALCHEMY): $(INST)/$(ZODB)
should be removed completely.
comment:13 Changed 9 years ago by
You are right! I probably produced this a bit too close to midnight. I will up a corrected patch shortly.
comment:14 Changed 9 years ago by
Patch updated.
comment:15 Changed 9 years ago by
- Description modified (diff)
comment:16 Changed 9 years ago by
- Status changed from needs_review to needs_work
- Work issues set to sagenb
This causes build problems for sagenb:
Processing dependencies for Twisted==12.1.0 Searching for zope.interface Link to http://pypi.python.org/simple/zope.interface/ ***BLOCKED*** by --allow-hosts Couldn't find index page for 'zope.interface' (maybe misspelled?) Scanning index of all packages (this may take a while) Link to http://pypi.python.org/simple/ ***BLOCKED*** by --allow-hosts No local packages or download links found for zope.interface error: Could not find suitable distribution for Requirement.parse('zope.interface') Error installing Twisted-12.1.0.tar.bz2 !
comment:17 Changed 9 years ago by
I think this was expected and is https://github.com/sagemath/sagenb/issues/126. So we should open a ticket on Sage's trac and put it as a dependency here I guess.
comment:18 Changed 9 years ago by
- Dependencies changed from #12205 to #12205, sagenb-???
- Milestone changed from sage-5.6 to sage-pending
- Status changed from needs_work to needs_review
comment:19 Changed 9 years ago by
- Dependencies changed from #12205, sagenb-??? to #12205, #13717
comment:20 Changed 9 years ago by
I think this is already fixed, we just need to make a new sagenb tarball. (The only thing sagenb needs to do is package zope.interface, right?)
comment:21 Changed 9 years ago by
comment:22 Changed 9 years ago by
And now that I look more closely at this ticket's dependencies, Jeroen seems to have already noticed that... sorry for the noise :)
comment:23 Changed 9 years ago by
- Work issues sagenb deleted
comment:24 Changed 9 years ago by
- Dependencies changed from #12205, #13717 to #12205, #13717, #13963
comment:25 Changed 9 years ago by
I don't feel like reviewing the Sage library patch, but it builds and works fine now.
comment:26 Changed 9 years ago by
It's just removing files and one line that is useless. Christopher, could you give a review to this?
comment:27 Changed 9 years ago by
- Milestone changed from sage-pending to sage-5.7
- Reviewers set to Jeroen Demeyer
- Status changed from needs_review to positive_review
comment:28 Changed 9 years ago by
- Merged in set to sage-5.7.beta1
- Resolution set to fixed
- Status changed from positive_review to closed
Please do not feel offended of my naive question, but what do you think of completely removing ZODB? As far as I understand (by looking at the failing doctests when removing ZODB), only Cremona's database and the Conway polynomials database use it. Ticket #11587 migrates the Cremona's DB to SQLite, so would it not be possible to the same with the Polynomial DB? Are there any optional SPKGs which do depend on ZODB?