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

Allow setting the attach timeout #5

Merged
merged 2 commits into from Apr 17, 2012
Merged

Conversation

daf
Copy link
Contributor

@daf daf commented Apr 17, 2012

I ran into some problems while trying to profile long-running operations - whenever my code eventually got to the detach method, it would just throw a python error. Tracing the steps, I figured it out there is a timeout involved, by default set to 60 seconds.

This patch does a few things:

  • Makes it so calling detach when there isn't an active attach session is not an error
  • Allows you to configure or disable the timeout by calling set_attach_duration - set to 0 to disable.

@srlindsay
Copy link
Owner

Seems fine to me. I don't use attach directly in applications that I profile, instead using attach_on_signal which takes the profile duration as an argument. I suppose the signal-based trigger only makes sense if you're profiling a large amount of short tasks, rather than one long-running task.

srlindsay added a commit that referenced this pull request Apr 17, 2012
Allow setting the attach timeout
@srlindsay srlindsay merged commit 2d9af68 into srlindsay:master Apr 17, 2012
@srlindsay
Copy link
Owner

So, the _attach_duration global was there only to allow attach_on_signal to work. Setting the duration makes way more sense as an argument to attach.

Take a look at the no_global_duration branch -- if that will work for you use case, I think I'd prefer to go that route, getting rid of _attach_duration and set_attach_duration.

@daf
Copy link
Contributor Author

daf commented Apr 18, 2012

Yes, that looks great to me. Seems to be working fine here.

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.

None yet

2 participants