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

MPP-3487 - Fix tests, handle missing sqlcommenter in Python 3.12, #4724

Merged
merged 4 commits into from
May 29, 2024

Conversation

jwhitlock
Copy link
Member

This makes changes needed for tests to run under Python 3.12, with no deprecation warnings.

Mock that doesn't do what was intended

For test_grab_keyfile_checks_cert_url_origin, the line

assert mock_urlopen.called_once_with(cert_url)

raises an exception:

AttributeError: 'called_once_with' is not a valid assertion. Use a spec for the mock if 'called_once_with' is meant to be an attribute.. Did you mean: 'assert_called_once_with'?

This is a new check for mock functions which are similar to the mock assertion functions. The developer probably intended:

mock_urlopen.assert_called_once_with(cert_url)

However, this also fails. It would be a bit of work to make this pass, requiring mocking the return of a valid PEM. After a bit of reading, I re-wrote as:

mock_urlopen.assert_not_called()

and called it after the SuspiciousOperation was raised.

LogRecord has new attribute taskName

This attribute was added to log the asyncio task name, python/cpython#91513 . The log_extra helper needs to ignore it.

utcnow() is finally going away

datetime.utcnow() is one of the gotcha methods. It returns a datetime in UTC time, but without timezone information. Django will catch this if you try to store in a model, but some of our test code still uses it. It now raises a DeprecationWarning. I've replaced this code with datetime.now(UTC), which returns a timezone-aware datetime.

google-cloud-sqlcommenter doesn't like Python 3.12

This module prints a bunch of DeprecationWarnings: in Python 3.11

/private/tmp/workspace/.py311/lib/python3.11/site-packages/google/__init__.py:17: DeprecationWarning: pkg_resources is deprecated as an API. See https://setuptools.pypa.io/en/latest/pkg_resources.html
  __import__('pkg_resources').declare_namespace(__name__)
/private/tmp/workspace/.py311/lib/python3.11/site-packages/google/__init__.py:17: DeprecationWarning: Deprecated call to `pkg_resources.declare_namespace('google')`.
Implementing implicit namespace packages (as specified in PEP 420) is preferred to `pkg_resources.declare_namespace`. See https://setuptools.pypa.io/en/latest/references/keywords.html#keyword-namespace-packages
  __import__('pkg_resources').declare_namespace(__name__)

In Python 3.12, pkg_resources is no longer installed automatically in virtualenvs, and these become an error:

Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/private/tmp/workspace/.py312/lib/python3.12/site-packages/google/__init__.py", line 17, in <module>
    __import__('pkg_resources').declare_namespace(__name__)
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^
ModuleNotFoundError: No module named 'pkg_resources'

You can install it via setuptools, but installing this unconditionally will cause issues when you are not in a virtualenv, like our Docker image. I went with skipping it for Python 3.12, and not installing the middleware if the import fails (either not installed, or installed in virtualenv without setuptools). I opened google/sqlcommenter#275 to track the issue.

Copy link
Collaborator

@say-yawn say-yawn left a comment

Choose a reason for hiding this comment

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

Changes have very good documentation and commit message. LGTM 🚀

@jwhitlock jwhitlock added this pull request to the merge queue May 29, 2024
jwhitlock added a commit that referenced this pull request May 29, 2024
MPP-3487 - Fix tests, handle missing sqlcommenter in Python 3.12,
Merged via the queue into main with commit a754c66 May 29, 2024
29 checks passed
@jwhitlock jwhitlock deleted the python-3-12-tests-MPP-3487 branch May 29, 2024 19:45
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.

2 participants