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

fix perl regex to calculate download traffic per day correctly on GHE 2.12.x #107

Closed
wants to merge 4 commits into from

Conversation

rashamalek
Copy link
Contributor

this is tested on GHE 2.12.3, and covers: #98
Please test it on GHE 2.11.x if you have any running instance.

Rasha Malek and others added 2 commits January 31, 2018 18:43
@@ -20,15 +20,20 @@ if ghe_greater_equal "2.11.0" ; then
# most recent log files (because the information from yesterday may or not
# be rotated already).
CAT_LOG_FILE="zcat -f /var/log/github-audit.{log.1*,log} | grep -F '$(date --date='yesterday' +'%b %_d')'"

# The order in github-audit.log in GHE 2.12.x has been changed, to pick the right order PERL_REGEX is created.
PERL_REGEX='print if s/.*"cloning":([^,]+).*"program":"upload-pack".*"repo_name":"([^"]+).*"uploaded_bytes":([^,]+).*"user_login":"([^"]+).*/\2\t\4\t\1\t\3/'
Copy link
Collaborator

Choose a reason for hiding this comment

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

Wouldn't that use the new regex for 2.11, too? The function above checks for greater_equal!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are right, then I recommend separating the version check for perl regex and the log file into 2 different ifs.

Rasha Malek and others added 2 commits January 31, 2018 19:03
@pluehne pluehne added the bug label Jan 31, 2018
Copy link
Contributor Author

@rashamalek rashamalek left a comment

Choose a reason for hiding this comment

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

@larsxschneider do you think this way we could cover the concern?

@larsxschneider
Copy link
Collaborator

larsxschneider commented Feb 4, 2018

Sorry @rashamalek for the delay. Thanks a lot for this fix 👍 - this is awesome 🎉 !

I reformatted your patch a bit and squashed it as 1e2e268 !

@rashamalek
Copy link
Contributor Author

👍

@larsxschneider
Copy link
Collaborator

I just realized that you don't show up in the contributors graph. If think you need to add your zalando email address to your github profile to make this work because I squashed the commits and didn't merge the PR directly. Sorry!

@rashamalek
Copy link
Contributor Author

No Worries :), I added the email.

@pluehne
Copy link
Contributor

pluehne commented Feb 5, 2018

@rashamalek: And thanks for spotting this and submitting the fix 😄!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants