Skip to content

Reimplement HIDE_IN_STACKTRACES machinery #1635

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

Merged
merged 3 commits into from
May 31, 2022

Conversation

living180
Copy link
Contributor

The pre-existing HIDE_IN_STACKTRACES machinery was based on file system paths, but was rather inefficient because it required a file system operation (os.path.realpath()) for each stack frame. In addition, it would not work properly for namespace packages since they can have multiple file system hierarchies.

The new implementation avoids using paths altogether and instead uses the __name__ variable found in each frame's f_globals attribute to determine the associated module name, and compares that directly with the list of module names provided in the HIDE_IN_STACKTRACES setting.

living180 added 2 commits May 30, 2022 23:48
Prior to this commit, the HIDE_IN_STACKTRACES setting was implemented by
importing the modules listed in the setting and recording their file
system paths.  Then tidy_stacktrace() and
_StackRecorder.get_stack_trace() would look up the file system path for
each frame's code object and compare that path against the paths for the
excluded modules to see if it matched.  If so, the frame would be
excluded.

This was inefficient since it used a file system access,
os.path.realpath(), for each frame (although the _StackRecorder
implementation included some caching to reduce the cost).  It also would
not work correctly for namespace packages since they can have multiple
file system hierarchies.

Replace with a new implementation that instead retrieves the __name__
variable from the frame's f_globals attribute and matches that module
name against the list of excluded modules directly.
@living180 living180 closed this May 31, 2022
@living180 living180 deleted the frame_skipping branch May 31, 2022 11:07
@living180 living180 restored the frame_skipping branch May 31, 2022 11:08
@living180
Copy link
Contributor Author

I accidentally closed this PR by renaming the branch in my repo. Re-opening.

@living180 living180 reopened this May 31, 2022
Copy link
Member

@matthiask matthiask left a comment

Choose a reason for hiding this comment

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

Thanks! Looks good.

I think I'd prefer both get_stack_trace methods to use keyword-only arguments at that point in time. It should make future refactorings easier, I hope.

@living180
Copy link
Contributor Author

I think I'd prefer both get_stack_trace methods to use keyword-only arguments at that point in time. It should make future refactorings easier, I hope.

Done.

@matthiask matthiask merged commit 2cd4a02 into django-commons:main May 31, 2022
@matthiask
Copy link
Member

Thanks!

@living180 living180 deleted the frame_skipping branch May 31, 2022 20:06
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