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 string interpolation cache api and related specs #4325

Closed

Conversation

colinsurprenant
Copy link
Contributor

  • adds the cache_size and clear_cache methods to StringInterpolation
  • use these methods in specs
  • support methods in the Java Event

Fixes #4249 and relates to #4191 and #4229


# clear the global compiled templates cache
def clear_cache
Java::ComLogstash::StringInterpolation.get_instance.clear_cache;
Copy link
Contributor

Choose a reason for hiding this comment

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

Style question - any reason you chose this syntax instead of com.logstash.StringInterpolation ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe this is the preferred way of referencing Java classes, the other notations AFAIU is only for top level Java packages java, javax, org, and com per https://github.com/jruby/jruby/wiki/CallingJavaFromJRuby#referencing-java-classes-using-full-qualified-class-name

@jordansissel
Copy link
Contributor

I only have style concerns. LGTM. The style points I commented on can be resolved at a later date - I have no preference. Merging. :)

@elasticsearch-bot
Copy link

Jordan Sissel merged this into the following branches!

Branch Commits
master aaaa6de, 6a3e021

@colinsurprenant
Copy link
Contributor Author

perfect, thanks!

colinsurprenant added a commit to colinsurprenant/logstash that referenced this pull request Dec 10, 2015
colinsurprenant added a commit to colinsurprenant/logstash that referenced this pull request Dec 10, 2015
@colinsurprenant
Copy link
Contributor Author

this also need to be merged into 2.x, will do

colinsurprenant added a commit that referenced this pull request Dec 10, 2015
colinsurprenant added a commit that referenced this pull request Dec 10, 2015
colinsurprenant added a commit to colinsurprenant/logstash that referenced this pull request Jan 12, 2016
colinsurprenant added a commit to colinsurprenant/logstash that referenced this pull request Jan 12, 2016
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.

3 participants