Skip to content
This repository was archived by the owner on Dec 14, 2023. It is now read-only.

Avoid using lock in emit - #29

Merged
StephenHynes7 merged 1 commit into
LogentriesCommunity:masterfrom
graingert:avoid-using-lock-in-LE
Feb 24, 2015
Merged

Avoid using lock in emit#29
StephenHynes7 merged 1 commit into
LogentriesCommunity:masterfrom
graingert:avoid-using-lock-in-LE

Conversation

@graingert

Copy link
Copy Markdown
Contributor

No description provided.

Comment thread logentries/utils.py

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This lock is causing emit to be synchronised globally which is ineficient.

Because the lock is only to avoid double-starting the thread, and python checks this for us in threading, we can simply try to start it and catch the exception.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Sounds good, ill merge this in now.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hi @graingert. I agreen with your arguments.
But, this change prevent the error, RuntimeError Exception threads can only be started once?
This error will now be handled by the logger of django for example?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

No it won't be logged because I've caught the exception.

StephenHynes7 pushed a commit that referenced this pull request Feb 24, 2015
@StephenHynes7
StephenHynes7 merged commit c884e09 into LogentriesCommunity:master Feb 24, 2015
StephenHynes7 pushed a commit that referenced this pull request Feb 25, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants