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

Make log.exception the default but with a way to silence it #158

Open
WilliamStam opened this issue Jun 15, 2022 · 1 comment
Open

Make log.exception the default but with a way to silence it #158

WilliamStam opened this issue Jun 15, 2022 · 1 comment

Comments

@WilliamStam
Copy link

WilliamStam commented Jun 15, 2022

EDIT: See the comment bellow. I think removing the logging altogether is the way to go. Its only being used in 1 place.

It would be pretty cool if there was a way to "silence" the log exception in the library. For instance if the app logs all exceptions to DB / log file / external system then having the library log.exception(f"aio process {os.getpid()} failed") results in a double log with a ton of "aio process failed" ones which arent as useful if you are trying to handle the exceptions appropriately.

I propose that the library get a flag that can be set to log_exception which by default is True (backwards compatibility) and if false then it wont log.exception(f"aio process {os.getpid()} failed") it will simply raise the error like normal

the pool worker has an exception handler but the Worker doesn't altho im not sure if that matters much to this suggestion.

  • OS: Windows / Ubuntu
  • Python version: 3.10
  • aiomultiprocess version: 0.9.0

test case

import asyncio
import logging
import aiomultiprocess

logger = logging.getLogger(__name__)

class TestException(Exception): pass

async def run_worker():
    raise TestException("woof")

async def main():
    print("here we go")
    try:

        result = await aiomultiprocess.Worker(
            target=run_worker
        )
        if isinstance(result,BaseException):
            raise result.with_traceback(result.__traceback__)

    except TestException as e:
        logger.exception(e)
    except BaseException:
        print("BaseException","-"*100)
    except Exception as e:
        print("Exception","-"*100)
    except:
        print("this is bad","-"*100)

if __name__ == '__main__':
    try:
        asyncio.run(main())
    except Exception as e:
        logger.exception(e)
    except KeyboardInterrupt:
        logger.info("KeyboardInterrupt in root")
    finally:
        print("Shut Down")

Expectation: to not log the aio process 11092 failed exception. to only have the TestException and not both

Fix: let the developer set a flag to log_exception to False for it to simply not log.exception it.

workaround: setting a custom logging handler that ignores log messages that match "aio process xxxx failed" which im sure is a really bad idea.

@WilliamStam
Copy link
Author

or to continue with #151 and remove it alltogether. i would be happy if it was removed. so far the only place logging is used in core is in

@staticmethod
    def run_async(unit: Unit) -> R:
...
        except BaseException:
            log.exception(f"aio process {os.getpid()} failed")
            raise

@WilliamStam WilliamStam changed the title make log.exception the default but with a way to silence it Make log.exception the default but with a way to silence it Jun 15, 2022
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

No branches or pull requests

1 participant