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

# 110 Globs that Return Subdirectories Error out #111

Closed

Conversation

original-brownbear
Copy link
Contributor

closes #110

@original-brownbear
Copy link
Contributor Author

@guyboertje fyi :)

@original-brownbear
Copy link
Contributor Author

@guyboertje moved to Stud::Temporary as discussed :)

@suyograo
Copy link
Contributor

suyograo commented May 9, 2017

Nice. LGTM

@suyograo suyograo self-requested a review May 9, 2017 19:58
@suyograo
Copy link
Contributor

suyograo commented May 9, 2017

@original-brownbear remember to bump version, changelog and publish :)

@suyograo
Copy link
Contributor

suyograo commented May 9, 2017

@original-brownbear since you are in this area, this is another candidate for a java rewrite (should continue using Joni to keep backward compatibility with regexes). Something on your list if you are looking for more things :)

It would be interesting to see what the benchmark results are for the 2 implementations.

/cc @jsvd

@original-brownbear
Copy link
Contributor Author

@suyograo bump version in this PR or are we always doing a separate one for that?

@original-brownbear
Copy link
Contributor Author

@suyograo Also, I'll add a separate issue for a Java redo then?

# A hash of pattern-name and pattern tuples defining custom patterns to be used by
# the current filter. Patterns matching existing names will override the pre-existing
# definition. Think of this as inline patterns available just for this definition of
# A hash of pattern-name and pattern tuples defining custom patterns to be used by
Copy link
Contributor

Choose a reason for hiding this comment

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

Please disable your editor's "strip trailing whitespace" feature.

@@ -390,7 +390,7 @@ def patterns_files_from_paths(paths, glob)

Dir.glob(path).each do |file|
@logger.trace("Grok loading patterns from file", :path => file)
patternfiles << file
patternfiles << file unless File.directory?(file)
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's log when we are skipping things. Perhaps at debug level, logger.debug("Skipping path because it is a directory", :path => file)

@original-brownbear
Copy link
Contributor Author

@jordansissel all fixed I think :)

@suyograo
Copy link
Contributor

suyograo commented May 9, 2017

@original-brownbear you can bump version and changelog in the same PR.

@original-brownbear
Copy link
Contributor Author

@suyograo bumped version and added changelog entry :)

@original-brownbear
Copy link
Contributor Author

issue about Java version: #112

@elasticsearch-bot
Copy link

Armin Braun merged this into the following branches!

Branch Commits
master 8c32f3d, 4527ef7, 7291a48, e5f3ee9, 1f6d1c2, ccf7cb4, 58db896, e5d432f

elasticsearch-bot pushed a commit that referenced this pull request May 9, 2017
elasticsearch-bot pushed a commit that referenced this pull request May 9, 2017
elasticsearch-bot pushed a commit that referenced this pull request May 9, 2017
elasticsearch-bot pushed a commit that referenced this pull request May 9, 2017
elasticsearch-bot pushed a commit that referenced this pull request May 9, 2017
elasticsearch-bot pushed a commit that referenced this pull request May 9, 2017
elasticsearch-bot pushed a commit that referenced this pull request May 9, 2017
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.

Directory under patterns directory can crash logstash at startup
5 participants