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

fancylogger raiseException support templaiting via args #233

Open
stdweird opened this issue Jul 26, 2016 · 8 comments
Open

fancylogger raiseException support templaiting via args #233

stdweird opened this issue Jul 26, 2016 · 8 comments

Comments

@stdweird
Copy link
Member

raiseException("%s %s", "a", "b", Exception) should do the right thing

@stdweird
Copy link
Member Author

@JensTimmerman toughts?

@JensTimmerman
Copy link
Contributor

personally I would want to remove the raiseException call alltogether. it's not the loggers work to raise exceptions, only to log them.

@wdpypere
Copy link
Contributor

wdpypere commented Apr 3, 2021

I ran into this issue and I agree with @JensTimmerman on this. We should deprecate raiseException and start logging deprecation warnings.

@wdpypere
Copy link
Contributor

wdpypere commented Apr 3, 2021

if we don't deprecate it, it should work like all logger functions.

the quick example is:

>>> from vsc.utils import fancylogger
>>> logger = fancylogger.getLogger()
>>> logger.raiseException('test %s%s', "hello", "world")
2021-04-03 13:23:40,401 WARNING    <stdin>         MainThread  test %s%s
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/wdpypere/.local/lib/python3.9/site-packages/vsc_base-3.2.0-py3.9.egg/vsc/utils/fancylogger.py", line 346, in raiseException
    raise_with_traceback(exception(message))
TypeError: 'str' object is not callable

@stdweird
Copy link
Member Author

stdweird commented Apr 3, 2021

@wdpypere the problem is that people will only raise the exception (which doesn't support the lazy passing of args), and not log, and this makes most of the exceptions problematic to debug.

@wdpypere
Copy link
Contributor

wdpypere commented Apr 3, 2021

I see the issue, but there is nothing preventing users from doing try ... except Exception ... pass either, which is arguably even worse. I don't know how far we want to/should go to protect people of shooting themselves in the foot.

but then raiseException should at least work as all the other logging functions. I'll have a go at it.

@wdpypere
Copy link
Contributor

wdpypere commented Apr 3, 2021

looking at it, its (as expected) a quite complex problem. As I can see in our code there are 2 main uses for raiseException:

  1. from inside an except block when there is already an Exception
  2. when one wants to create an exception

If we really want to maintain a hybrid error/log thing I would do the same as python itself does it. logging.exception is no more than:

def exception(self, msg, *args, exc_info=True, **kwargs):
    self.error(msg, *args, exc_info=exc_info, **kwargs)

we could do that with an extra raise included, but that solution would only work for case 1, not for case 2.

@stdweird
Copy link
Member Author

stdweird commented Apr 3, 2021

i wouldn't spend too much time on it
i just think that having a method for it is better than the all the copy paste, but this breadks the upstream logging module and somewhat hides raises.

but i'm ok with deprecating it.

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

3 participants