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

Change logging so that the messages appear once on thread status change #36

Merged

Conversation

artemrizhov
Copy link
Collaborator

Closes #21.

@niccokunzmann
Copy link
Owner

You reordered the functions so I can not really see what changed. Would you like to merge it like this or create two commits?

thread.daemon = True
thread.start()
return thread
def write_log(title, message=''):
Copy link
Owner

Choose a reason for hiding this comment

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

I like this: It is a basis of setting a log

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I thought somebody may want to replace this function with own implementation to write messages to another destination in another format.

@artemrizhov
Copy link
Collaborator Author

Ok, no problem. I just found that we lost structure.

@artemrizhov artemrizhov force-pushed the avoid_repeating_messages branch from 4480fad to 6b28e3a Compare January 21, 2017 21:08
@artemrizhov
Copy link
Collaborator Author

I've changed the order back so that you can review the changes. I'll create another PR for with reordered functions then.

# Gotcha!
hanging_threads.add(thread_id)
# Report the hanged thread.
log_hanged_thread(thread_id, frame)
old_threads = new_threads
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I have to say that the monitor() function becomes too large. However I have not found simple elegant way to divide it into small chunks. I think it should be rewritten in FP or OOP style. But this is probably a job for another task. For now I just want to make it usable for me and others to let us all fix the hanging threads problem in our apps :)

Copy link
Owner

Choose a reason for hiding this comment

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

You can create an issue: refactor monitor function and an issue to write tests.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Created #40 and #41.

@niccokunzmann niccokunzmann merged commit 90f972b into niccokunzmann:master Jan 21, 2017
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