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

Call engine.logger.* hooks around logging a message #1129

Merged
merged 1 commit into from
Feb 9, 2016

Conversation

veryrusty
Copy link
Member

The hook names were defined, but they were never called.

Call them in an around modifier on the log method (similar to where session engine hooks are called). Either being lazy or wise; these hooks are passed the logging engine object, and all params to the log method.

Tests include an example of changing the log level before the message is passed to the logger.

The hook names were defined, but they were never called. Call them in an
anround modifier on the log method (similar to where session engine
hooks are called). Either being lazy or wise; these hooks are passed the
logging engine object, and all params to the log method.

Tests include an example of changing the log level before the message is
passed to the logger.
@SysPete
Copy link
Member

SysPete commented Feb 9, 2016

👍 I'm thinking of some really weird ways to use this - like it a lot.

@xsawyerx
Copy link
Member

xsawyerx commented Feb 9, 2016

👍 Nice!

@cromedome
Copy link
Contributor

👍 That's awesome. Like @SysPete, I am anxious to find ways to (ab)use this!

@veryrusty veryrusty merged commit c56951f into master Feb 9, 2016
@veryrusty
Copy link
Member Author

Merged. Thanks everyone for taking a look! My first plan for this in untangling D2::P::LR (per #1125).

@xsawyerx
Copy link
Member

@veryrusty Can you detail some of the plan?

@veryrusty
Copy link
Member Author

D2::P::LR does two things

  • puts logged messages into the session => look at doing this in engine.logger.before (but can stay where it is), and
  • in some cases redirects to / => move this to an engine.logger.after hook (which separates concerns between logging and doing something in dancer).

xsawyerx added a commit that referenced this pull request Apr 19, 2016
    [ BUG FIXES ]
    * GH #1102: Handle multiple '..' in file path utilities.
      (Oleg A. Mamontov, Peter Mottram)
    * GH #1114: Fix missing prereqs as reported by CPANTS.
      (Mohammad S Anwar)
    * GH #1128: Shh warning if optional megasplat is not present.
      (David Precious)
    * GH #1139: Fix incorrect Content-Length header added by AutoPage
      handler (Michael Kröll, Russell Jenkins)
    * GH #1144: Change tt tags to span in skel (Jason Lewis)
    * GH #1046: "no_server_tokens" configuration option doesn't work.
      (Sawyer X)
    # GH #1155, #1157: Fix megasplat value splitting when there are empty
      trailing path segments. (Tatsuhiko Miyagawa, Russell Jenkins)
      NOTE: Paths matching a megasplat that end with a '/' will now include
      an empty string as the last value. For the route pattern '/foo/**',
      the path '/foo/bar', the megasplat gives ['bar'], whereas '/foo/bar/'
      now gives ['bar','']. Joining the array of megasplat values will now
      always be the string matched against for the megasplit.

    [ DOCUMENTATION ]
    * GH #1119: Improve the deployment documentation. (Andrew Beverley)
    * GH #1123: Document import of utf8 pragma. (Victor Adam)
    * GH #1132: Fix spelling mistakes in POD (Gregor Herrmann)
    * GH #1134: Fix spelling errors detected by codespell (James McCoy)
    * GH #1153: Fix POD rendering error. (Sawyer X)

    [ ENHANCEMENTS ]
    * GH #1129: engine.logger.* hooks are called around logging a message.
      (Russell @veryrusty Jenkins)
    * GH #1146: Cleaner display of error context (Vernon Lyon)
    * GH #1085: Add consistent keywords for accessing headers;
      'request_header' for request, 'response_header', 'response_headers'
      and 'push_response_header' for response. (Russell @veryrusty Jenkins)
@xsawyerx xsawyerx deleted the feature/logger_hooks branch September 26, 2016 22:33
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