LogParser can accept directories, now #126

Merged
merged 2 commits into from Oct 7, 2012

Conversation

Projects
None yet
2 participants
@spikegrobstein
Contributor

spikegrobstein commented Oct 4, 2012

If it encounters a directory in the argument list, it will recursively
go through the files in it and parse them. (no tests for this use-case)

I couldn't figure out how to test this in such a way that fits in with the rest of your test suite, but some quick tests looked like it worked.

I had to process through tens of thousands of S3 logs and didn't have the time or processing power to combine them. This change saved me HOURS.

LogParser can accept directories, now
If it encounters a directory in the argument list, it will recursively
go through the files in it and parse them. (no tests for this use-case)

@ghost ghost assigned barttenbrinke Oct 5, 2012

@barttenbrinke

This comment has been minimized.

Show comment Hide comment
@barttenbrinke

barttenbrinke Oct 5, 2012

Collaborator

Hey Spike,
thats very nice addition! Thank you for contributing.
But as you said, it does need a test.
You could test it quite simply by adding a subdirectory to the fixtures directory named something like "log_directory" that contains two or three s3 sample files containing two or three lines each.
The test itself can be added to the integration/command_line_usage_spec, something like:

output = run("#{log_directory_fixture} --format s3")
output.any? { |line| /^Parsed requests:\s*12\s/ =~ line }.should be_true

You can add the log_directory_fixture function in helpers.rb and have it point to the created directory.

Greetings, Bart

Collaborator

barttenbrinke commented Oct 5, 2012

Hey Spike,
thats very nice addition! Thank you for contributing.
But as you said, it does need a test.
You could test it quite simply by adding a subdirectory to the fixtures directory named something like "log_directory" that contains two or three s3 sample files containing two or three lines each.
The test itself can be added to the integration/command_line_usage_spec, something like:

output = run("#{log_directory_fixture} --format s3")
output.any? { |line| /^Parsed requests:\s*12\s/ =~ line }.should be_true

You can add the log_directory_fixture function in helpers.rb and have it point to the created directory.

Greetings, Bart

@spikegrobstein

This comment has been minimized.

Show comment Hide comment
@spikegrobstein

spikegrobstein Oct 5, 2012

Contributor

Cool. Working on it. Thanks for the pointers. This is a lot easier than what I was trying to get working before.

I actually came across a couple of issues with my implementation while writing the tests so I'm working on fixing those and will update the pull request when I get that working.

Contributor

spikegrobstein commented Oct 5, 2012

Cool. Working on it. Thanks for the pointers. This is a lot easier than what I was trying to get working before.

I actually came across a couple of issues with my implementation while writing the tests so I'm working on fixing those and will update the pull request when I get that working.

full support for directories as logfile parameters
 - fixed a bug where it would throw an exception after finishing
   processing a directory.
 - working tests
@spikegrobstein

This comment has been minimized.

Show comment Hide comment
@spikegrobstein

spikegrobstein Oct 6, 2012

Contributor

This last commit should tie up all the loose ends.

Let me know if there's anything else I should do. Thanks!

Contributor

spikegrobstein commented Oct 6, 2012

This last commit should tie up all the loose ends.

Let me know if there's anything else I should do. Thanks!

barttenbrinke added a commit that referenced this pull request Oct 7, 2012

Merge pull request #126 from spikegrobstein/feature/log_directory
Rla now accepts directories containing log files.

@barttenbrinke barttenbrinke merged commit 3a5f0f7 into wvanbergen:master Oct 7, 2012

1 check passed

default The Travis build passed
Details
@barttenbrinke

This comment has been minimized.

Show comment Hide comment
@barttenbrinke

barttenbrinke Oct 7, 2012

Collaborator

Hi Spike,

very nice work! Thanks for your contribution!

Greetings, Bart

Collaborator

barttenbrinke commented Oct 7, 2012

Hi Spike,

very nice work! Thanks for your contribution!

Greetings, Bart

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment