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

Python 3.13 Compatibility (cgi, dbm.sqlite) #978

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

ephes
Copy link

@ephes ephes commented Feb 13, 2025

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:

What your changes do and why you chose this solution

This PR removes remaining usages of the deprecated cgi module and uses dbm.gnu as the default dbm backend which is used by _shelve_compat.

Checklist

  • Checked that no other issues or pull requests exist for the same issue/change
  • Added tests covering the new functionality
  • Updated documentation OR the change is too minor to be documented
  • Updated CHANGELOG.md OR changes are insignificant

Hmm, this is actually kind of hard to test..

@kushaldas
Copy link

I started looking for 3.13 and pysaml2 and saw this PR opened 1 hour ago :)

"""
import dbm.gnu
import dbm
dbm._defaultmod = dbm.gnu
Copy link

@kushaldas kushaldas Feb 13, 2025

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.

Copy link
Author

@ephes ephes Feb 14, 2025

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)

Copy link
Author

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)

Copy link
Member

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

  1. users that have used pysaml2 and this backend before and have a file available.
  2. 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?

Copy link
Author

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.

@kushaldas
Copy link

Related thread: python/cpython#100414

Co-authored-by: Ivan Kanakarakis <ivan.kanak@gmail.com>
@ephes ephes requested a review from kushaldas February 20, 2025 09:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Python 3.13 Compatibility
3 participants