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

Exit with an error if --gil but we failed to get necessary addrs/offsets #361

Merged
merged 3 commits into from
Mar 21, 2021

Conversation

Jongy
Copy link
Contributor

@Jongy Jongy commented Mar 19, 2021

Previously we'd just emit a warning, which is less noticeable; and the result of
a --gil run where without the needed information is going to empty, anyway.

I think it's better, user-experience wise (like mentioned in #358 (comment)). Well at least I can vouch for it personally, I'm quite annoyed by --gil recordings producing empty stack files / graphs, it always takes me a short while to recall the possibility that it is simply unsupported...

About the positioning of the check - might've been nicer to place this code in main.rs, but it's much easier if the exit decision is made next to the failure itself. I can try moving it if you think that's better.

Previously we'd just emit a warning, which is less noticeable; and the result of
a --gil run where without the needed information is going to empty, anyway.
@Jongy Jongy mentioned this pull request Mar 19, 2021
Copy link
Owner

@benfred benfred left a comment

Choose a reason for hiding this comment

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

I think this is a good idea! thanks for adding,

src/python_spy.rs Outdated Show resolved Hide resolved
@benfred benfred merged commit 3322942 into benfred:master Mar 21, 2021
@Jongy Jongy deleted the exit-on-no-gil-detection branch March 21, 2021 17:15
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