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 __getattr__ conflict in prefect.docker.__init__ #16172

Merged
merged 4 commits into from
Dec 3, 2024
Merged

Conversation

zzstoatzz
Copy link
Collaborator

@zzstoatzz zzstoatzz commented Dec 2, 2024

closes #16171


Fix unittest.assertWarns() compatibility with module __getattr__ handling in prefect.docker.__init__

issue arises bc unittest's assertWarns uses getattr() with a default value to safely check for __warningregistry__ on modules

https://github.com/python/cpython/blob/e546876d83e1cc3f761e1eeca57f8a4533d06d00/Lib/unittest/case.py#L290-L294

Our __getattr__ implementation was raising ImportError, preventing Python from handling the default value case, so this PR changes it to raise AttributeError to follow attribute lookup protocol (like the other dynamic imports we have)

@zzstoatzz zzstoatzz changed the title let python handle itself fix getattr conflict in prefect.docker.__init__ Dec 2, 2024
Copy link

codspeed-hq bot commented Dec 2, 2024

CodSpeed Performance Report

Merging #16172 will not alter performance

Comparing getattr-docker (d219d60) with main (f1649e9)

Summary

✅ 3 untouched benchmarks

@zzstoatzz zzstoatzz changed the title fix getattr conflict in prefect.docker.__init__ fix __getattr__ conflict in prefect.docker.__init__ Dec 3, 2024
@zzstoatzz zzstoatzz added the fix A fix for a bug in an existing feature label Dec 3, 2024
@zzstoatzz zzstoatzz self-assigned this Dec 3, 2024
@zzstoatzz zzstoatzz marked this pull request as ready for review December 3, 2024 03:08
@github-actions github-actions bot added the bug Something isn't working label Dec 3, 2024
Copy link
Member

@desertaxle desertaxle left a comment

Choose a reason for hiding this comment

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

LGTM!

@zzstoatzz zzstoatzz merged commit 972ff69 into main Dec 3, 2024
51 checks passed
@zzstoatzz zzstoatzz deleted the getattr-docker branch December 3, 2024 03:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working fix A fix for a bug in an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ImportError when calling unittest.TestCase.assertWarns()
2 participants