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

Added filters, new nginx parser #9

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

Added filters, new nginx parser #9

wants to merge 10 commits into from

Conversation

bamse16
Copy link

@bamse16 bamse16 commented Jun 4, 2015

The main change is that I've added support for filters, so you can skip your 4xx status codes (which most of the times are related to bots scanning for phpmyadmin or such).

I am aware that's not the most optimal setup, and I'm open to suggestions. I'll be adding other filters, that are not that general (e.g. filter out js/css/etc requests), and it would be nice if everyone would be able to add their own parsers, without requiring others to run them.

@@ -187,9 +204,13 @@ def main(self):

# Abstract all the messy generator logic away into a simple for-each
for parsed_line in parsed_log_file:
if(self.should_filter(parsed_line)):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

any particular reason for the () around self.should_filter?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope, just that I'm new to python and a bit confused on some of it's syntax
:)

On Wed, Jun 10, 2015 at 6:46 AM, Andrew Gross notifications@github.com
wrote:

In parsible.py
#9 (comment):

@@ -187,9 +204,13 @@ def main(self):

     # Abstract all the messy generator logic away into a simple for-each
     for parsed_line in parsed_log_file:
  •        if(self.should_filter(parsed_line)):
    

any particular reason for the () around self.should_filter?


Reply to this email directly or view it on GitHub
https://github.com/Yipit/parsible/pull/9/files#r32061831.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No worries, overall this looks pretty good.

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.

None yet

2 participants