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

Give queue worker thread a name. #174

Merged
merged 2 commits into from
Nov 24, 2019
Merged

Give queue worker thread a name. #174

merged 2 commits into from
Nov 24, 2019

Conversation

t-mart
Copy link
Contributor

@t-mart t-mart commented Nov 23, 2019

Hey @Delgan,

I'm doing some debugging with a multi-threaded application and also have a Loguru sink with enqueue=True. (As we've discussed before) this instructs Loguru to manage a thread that processes a queue of log records.

Currently, that thread's name is the default one:

By default, a unique name is constructed of the form “Thread-N” where N is a small decimal number.

https://docs.python.org/3.8/library/threading.html#threading.Thread

For identification purposes, it'd be helpful to see a descriptive name for this thread in environments/debuggers/etc that support listing threads.

The name I've chosen loguru-writer.

This doesn't affect the operation of the Loguru in any other way:

[The name is] A string used for identification purposes only. It has no semantics.

https://docs.python.org/3.8/library/threading.html#threading.Thread.name

  • Give queue worker thread a name.
  • Add change to CHANGELOG.rst.
  • Tox passes.

@t-mart
Copy link
Contributor Author

t-mart commented Nov 23, 2019

Hmm. It seems I've caused code coverage to drop. Would you help me interpret why?

The drop seems to stem from a blank line?

@Delgan
Copy link
Owner

Delgan commented Nov 23, 2019

Good idea, thanks for the contribution!

What do you think of appending the handler _id to the thread name, like name="loguru-writer-%d" % self._id? This would give even more information, and avoid confusion if there are several running threads due to multiple sinks added with enqueue=true.

I don't know why coverage check failed... I encountered this problem once, on this same line. This line is executed in only one Travis build (Python 3.5.2), maybe there is a conflict with another build (Python 3.5). Particularly, the .travis.yml lists six builds while there is only five in the tox.ini file. Anyway, I restarted the build and it worked fine. 🙂

@t-mart
Copy link
Contributor Author

t-mart commented Nov 23, 2019

What do you think of appending the handler _id

Sounds good, as long as you are sure that will always be a successful interpolation (i.e. handlers always have a numeric _id attribute?).

I won't be able to add this change for probably around 24 hours. If you want to jump in and add it, no problem.

@Delgan
Copy link
Owner

Delgan commented Nov 23, 2019

Yeah, don't worry, I will update the PR if you don't have the time. 👍

@t-mart
Copy link
Contributor Author

t-mart commented Nov 24, 2019

@Delgan, the -<id> suffix has been added as you suggested.

@Delgan Delgan merged commit a5fc907 into Delgan:master Nov 24, 2019
@Delgan
Copy link
Owner

Delgan commented Nov 24, 2019

Great, thanks a lot!

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