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

logged_if_slow API #193

Closed
RazerM opened this issue Feb 17, 2016 · 5 comments
Closed

logged_if_slow API #193

RazerM opened this issue Feb 17, 2016 · 5 comments

Comments

@RazerM
Copy link
Collaborator

RazerM commented Feb 17, 2016

I think the logged_if_slow API is a bit awkard. I think we should change it for v1.0.

Current API:

def logged_if_slow(message, threshold=1, func=logbook_debug, args=None, kwargs=None)

Problems:

  • Always uses default logger
  • Have to pass log function, why not a level?
  • args and kwargs are normal parameters (rather than *args, **kwargs).

Proposed API:

def logged_if_slow(*args, **kwargs):
    threshold = kwargs.pop('threshold', 1)
    logger = kwargs.pop('logger', _default_logger)
    level = kwargs.pop('level', logbook.DEBUG)
    # Context manager would call:
    logger.log(level, *args, **kwargs)

Then the context manager can simply call

logger.log(level, *args, **kwargs)

Also: are there valid reasons to use threading and events to handle the threshold rather than just storing a time.time() value and comparing in __exit__? I don't know how the threading timeout works, so I'm just curious about the implementation!

@vmalloc
Copy link
Collaborator

vmalloc commented Feb 17, 2016

I'm not sure what you're asking - where did the message argument go in your proposal?

The idea behind using threads is to not be affected if the current thread is blocked on a long operation...

@RazerM
Copy link
Collaborator Author

RazerM commented Feb 17, 2016

message would be passed to logger.log as part of *args, just like the normal log methods. I've just edited the original message because I had *args in the wrong place.

@vmalloc
Copy link
Collaborator

vmalloc commented Feb 17, 2016

Your proposed syntax is illegal in Python 2, and would only work with using a single * in Python 3, so I'm not sure what you have in mind exactly...

@RazerM
Copy link
Collaborator Author

RazerM commented Feb 17, 2016

Sorry, the proposal is now valid syntax...

@vmalloc
Copy link
Collaborator

vmalloc commented Feb 17, 2016

Ok, seems reasonable. I prefer to have an optional func to preserve the current behavior and not break client code at least. Also - it must use threads IMO.

RazerM added a commit to RazerM/logbook that referenced this issue Feb 21, 2016
RazerM added a commit to RazerM/logbook that referenced this issue Feb 21, 2016
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

2 participants