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

ref(utils): Explicitly use None default when checking metadata #4039

Merged
merged 2 commits into from
Feb 11, 2025

Conversation

mpurnell1
Copy link
Contributor

@mpurnell1 mpurnell1 commented Feb 11, 2025

Fixes #4035

As described in the above issue, starting in Python 3.14 importlib_metadata 8 provides the desired behavior, raising KeyError on a missing key. In preparation for this change, and to remove a DeprecationWarning, we should explicitly default to None when getting metadata.

  • Tests
  • Linters
    • I encountered an issue while linting, inside importlib.metadata. It appears the type hint for Distribution.metadata shows it returning _meta.PackageMetadata, even though it actually returns _adapter.Message. I don't think that is within the scope of this PR, but I may bring it to their attention. Invalid linting error ignored with comment.

@sentrivana sentrivana added the Trigger: tests using secrets PR code is safe; run CI label Feb 11, 2025
@mpurnell1
Copy link
Contributor Author

@sentrivana I updated with master (I think I was missing some recent test fixes), can you re-add the tests trigger

Copy link

codecov bot commented Feb 11, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 80.24%. Comparing base (3217cca) to head (c6c42f3).
Report is 2 commits behind head on master.

✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4039      +/-   ##
==========================================
- Coverage   80.25%   80.24%   -0.01%     
==========================================
  Files         139      139              
  Lines       15403    15403              
  Branches     2605     2605              
==========================================
- Hits        12361    12360       -1     
  Misses       2202     2202              
- Partials      840      841       +1     
Files with missing lines Coverage Δ
sentry_sdk/utils.py 84.09% <100.00%> (ø)

... and 1 file with indirect coverage changes

@sentrivana
Copy link
Contributor

@mpurnell1 Thanks for updating, that should pull in fixes for the GraphQL test suites from master. You can ignore the AWS Lambda test failures on this PR, as long as the Common tests are passing and CI / Lint Sources is green, we're good to go.

That being said, the latter is failing because mypy is complaining about PackageMetadata having no .get. Like in most cases that have to do with mypy I'm not sure what's confusing since it isn't the case. Feel free to add a type: ignore[attr-defined] on the line to silence it.

@sentrivana sentrivana added the Trigger: tests using secrets PR code is safe; run CI label Feb 11, 2025
@sentrivana
Copy link
Contributor

Thank you!

@sentrivana sentrivana enabled auto-merge (squash) February 11, 2025 15:44
@sentrivana sentrivana merged commit c227e11 into getsentry:master Feb 11, 2025
152 of 154 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Trigger: tests using secrets PR code is safe; run CI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Prepare for importlib.metadata 8 update
2 participants