-
Notifications
You must be signed in to change notification settings - Fork 432
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
Python 3.13 Compatibility (cgi, dbm.sqlite) #978
base: master
Are you sure you want to change the base?
Conversation
I started looking for |
src/saml2/server.py
Outdated
""" | ||
import dbm.gnu | ||
import dbm | ||
dbm._defaultmod = dbm.gnu |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dbm._defaultmod = dbm.gnu
This should be removed. Other we get errors like following:
❯ python idp.py idp_conf
[2025-02-13 18:46:45,113] [DEBUG] [saml2.assertion.__init__] policy restrictions: None
[2025-02-13 18:46:45,113] [DEBUG] [saml2.assertion.__init__] policy restrictions: None
[2025-02-13 18:46:45,113] [DEBUG] [saml2.assertion.__init__] policy restrictions: {'default': {'lifetime': {'minutes': 15}, 'attribute_restrictions': None, 'name_form': 'urn:oasis:names:tc:SAML:2.0:attrname-format:uri', 'entity_categories': None}}
Traceback (most recent call last):
File "/home/kdas/code/learning/saml/pysaml2/example/idp2/idp.py", line 1076, in <module>
IDP = server.Server(args.config, cache=Cache())
File "/home/kdas/code/learning/saml/pysaml2/example/idp2/.venv/lib64/python3.13/site-packages/saml2/server.py", line 95, in __init__
self.init_config(stype)
~~~~~~~~~~~~~~~~^^^^^^^
File "/home/kdas/code/learning/saml/pysaml2/example/idp2/.venv/lib64/python3.13/site-packages/saml2/server.py", line 149, in init_config
idb = _shelve_compat(dbspec, writeback=True, protocol=2)
File "/home/kdas/code/learning/saml/pysaml2/example/idp2/.venv/lib64/python3.13/site-packages/saml2/server.py", line 71, in _shelve_compat
return shelve.open(name, *args, **kwargs)
~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/lib64/python3.13/shelve.py", line 250, in open
return DbfilenameShelf(filename, flag, protocol, writeback)
File "/usr/lib64/python3.13/shelve.py", line 227, in __init__
Shelf.__init__(self, dbm.open(filename, flag), protocol, writeback)
~~~~~~~~^^^^^^^^^^^^^^^^
File "/usr/lib64/python3.13/dbm/__init__.py", line 91, in open
raise error[0]("db type is {0}, but the module is not "
"available".format(result))
dbm.error: db type is dbm.gnu, but the module is not available
If we remove that, I can use the idp2
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you all think about these approaches?
- Switch to dbm.dump as the default across all Python versions
- Keep berkeleydb as default for Python <3.13, but use dbm.dump for 3.13+ (Side note: Anyone know what happened to berkeleydb support in 3.13?)
- The nuclear option: Scrap the _avoid_dbm_sqlite function entirely and tackle the underlying SQLite threading issues head-on (definitely the most challenging path)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I've updated the PR to use dbm.dumb
for Python 3.13+. Here are some approaches I tried that didn’t work out:
- Setting sqlite3.threadsafety = 2
- Using SRV = WSGIServer((HOST, PORT), application, numthreads=1)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should push everyone to use the new format. We have two cases
- users that have used pysaml2 and this backend before and have a file available.
- users that have not used pysaml2 or are switching to this backend for the first time.
If the file is not available, then we are on case (2) and we can just use Shelf with the new backend. If the file is there, we should try to open it with Shelf assuming it is an sqlite3 db. If that fails, we emit a warning and then convert it to be an sqlite3 db.
Doing the conversion at runtime could be expensive, but it will only be done once. To avoid paying this at runtime, we can provide update instructions with a script that will read the configuration, locate the database and convert it.
At a later update we can remove the conversion code.
Does that sound reasonable to you?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new format would be dbm.dumb
then, right? But I'm not really sure if it's a good choice, because of the fail safe issues mentioned in the related cPython-thread.
And the goal of having a conversion script would be that you could re-use your existing db when upgrading lets say from Python 3.12 to 3.13? I don't know if that's possible. It seems that it's not possible to read the Berkeley DB 1.85 files that are written by 3.12 and below by default when calling shelve.open
. At least not by using just the standard library.
Related thread: python/cpython#100414 |
Co-authored-by: Ivan Kanakarakis <ivan.kanak@gmail.com>
Closes #976
Description
I recently wrote a blog post demonstrating how to use Django as a service provider, with pysaml2 serving as an example IdP. A reader has since pointed out that it doesn’t work with Python 3.13.
The feature or problem addressed by this PR
After some investigation, I found two issues:
dbm.sqlite
being the new default dbm backend introduced in Python 3.13What your changes do and why you chose this solution
This PR removes remaining usages of the deprecated
cgi
module and usesdbm.gnu
as the default dbm backend which is used by_shelve_compat
.Checklist
Hmm, this is actually kind of hard to test..