Skip to content

Add "reinstall()" method to make it easier in spawn multiprocessing #1069

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

Merged
merged 3 commits into from
Feb 19, 2025

Conversation

monchin
Copy link
Contributor

@monchin monchin commented Jan 20, 2024

Make it easier when using loguru in spawn multiprocessing environment

@monchin
Copy link
Contributor Author

monchin commented Jan 20, 2024

It seems black does not check docstring's length.

@Delgan
Copy link
Owner

Delgan commented Jan 21, 2024

Thanks for the PR @monchin!

However, I'm going to wait a little before merging it, because there are quite a few special cases I'd like to think about and a lot of unit tests that need to be implemented. I'll probably make some commits after yours to complete the development, if you're OK with that.

It seems black does not check docstring's length.

Indeed, docstrings are not automatically formatted by black, we have to do it manually.

@monchin
Copy link
Contributor Author

monchin commented Jan 22, 2024

I'm not familiar with ruff, seems it failed because I created a new tests/test_reinstall.py file

@Delgan
Copy link
Owner

Delgan commented Jan 22, 2024

Thanks for the update.

I'm not familiar with ruff, seems it failed because I created a new tests/test_reinstall.py file

Similarly to black, the ruff tool notably replaces isort and is in charge of re-ordering the import statements.

You can try to install the pre-commit hooks to automatically fix your files before git commit:

$ pre-commit install --install-hooks

Or you can manually fix the file with:

$ ruff tests/test_reinstall.py --fix

@indigoviolet
Copy link

@Delgan What is the status of this? this would be a really nice addition to have.

gjhami added a commit to gjhami/LinkSiren that referenced this pull request Oct 31, 2024
Add multiprocessing functionality. However, this conflicts with wriring text to stdout other than a progressbar. Ideally, loguru could be used to manage logs in a multi-process safe way, but this requires a somewhat hacky solution of passing an attribute of the parent logger object into each spawned process and manually setting the logger imported in the spawned process to use the attribute passed as a paremeter. There is a [PR](Delgan/loguru#1069) to create a new function called reinstall() that would address this, but it has yet to be merged. Until the PR is merged, using multithreading instead of multiprocessing seems like the better solution.
gjhami added a commit to gjhami/LinkSiren that referenced this pull request Jan 20, 2025
Add multiprocessing functionality. However, this conflicts with wriring text to stdout other than a progressbar. Ideally, loguru could be used to manage logs in a multi-process safe way, but this requires a somewhat hacky solution of passing an attribute of the parent logger object into each spawned process and manually setting the logger imported in the spawned process to use the attribute passed as a paremeter. There is a [PR](Delgan/loguru#1069) to create a new function called reinstall() that would address this, but it has yet to be merged. Until the PR is merged, using multithreading instead of multiprocessing seems like the better solution.
@Delgan Delgan merged commit ab794fe into Delgan:master Feb 19, 2025
@Delgan
Copy link
Owner

Delgan commented Feb 19, 2025

Thanks for your contribution and sorry for the delay.

The logger.reinstall() method is a great addition. I'll merge the PR as-is and see if further refinements are needed (thinking about some edge cases).

Refs: #818 #912 #1064

@wynemo
Copy link

wynemo commented Mar 8, 2025

hello, guys, does it actually work?

run it on linux, it fails:

import multiprocessing
from loguru import logger

def subworker(_logger):
    _logger.reinstall()
    _logger.info("Child")


def test_process_spawn():
    spawn_context = multiprocessing.get_context("spawn")
    logger.add("file.log", context=spawn_context, enqueue=True, catch=False)

    process = spawn_context.Process(target=subworker, args=(logger,))
    process.start()
    process.join()
    assert process.exitcode == 0
    _logger.info("Main")
    _logger.remove()

if __name__ == "__main__":
    test_process_spawn()

ubuntu@oracle:/tmp$ ./venv/bin/python test.py
Traceback (most recent call last):
  File "/tmp/test.py", line 21, in <module>
    test_process_spawn()
  File "/tmp/test.py", line 14, in test_process_spawn
    process.start()
  File "/usr/lib/python3.10/multiprocessing/process.py", line 121, in start
    self._popen = self._Popen(self)
  File "/usr/lib/python3.10/multiprocessing/context.py", line 288, in _Popen
    return Popen(process_obj)
  File "/usr/lib/python3.10/multiprocessing/popen_spawn_posix.py", line 32, in __init__
    super().__init__(process_obj)
  File "/usr/lib/python3.10/multiprocessing/popen_fork.py", line 19, in __init__
    self._launch(process_obj)
  File "/usr/lib/python3.10/multiprocessing/popen_spawn_posix.py", line 47, in _launch
    reduction.dump(process_obj, fp)
  File "/usr/lib/python3.10/multiprocessing/reduction.py", line 60, in dump
    ForkingPickler(file, protocol).dump(obj)
TypeError: cannot pickle '_io.TextIOWrapper' object

am i missing something?

edit: never mind, just call logger.remove before test, because "sys.stderr" sink is not picklable

@monchin
Copy link
Contributor Author

monchin commented Mar 8, 2025

Thanks for your contribution and sorry for the delay.

Thank you for merging and it's my pleasure.

I wanted to make loguru name rotation files the same way as std logging when rotating every midnight, and found that actually I can use logger.add(f"file.log.{time:YYYY-MM-DD}). Now I can replace std logging with loguru from next release!

@Delgan
Copy link
Owner

Delgan commented Mar 16, 2025

@wynemo The new reinstall() method only helps with updating the global logger after it's inherited by a child process. However, this does not change the fact that the logger must be inheritable to begin with.

In your case, you're facing an error because the default sink, which is sys.stderr, can't be inherited (because it contains non-picklable attributes). This is a common problem. You need first to remove the default sink, then re-add it with enqueue=True (which ensures the sink can be inherited):

spawn_context = multiprocessing.get_context("spawn")
logger.remove()
logger.add(sys.stderr, context=spawn_context, enqueue=True)
logger.add("file.log", context=spawn_context, enqueue=True, catch=False)

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.

4 participants