Fixed Gemfile to permit development with local Logstash clone #82
Conversation
Gemfile
Outdated
|
|
||
| if Dir.exist?(logstash_path) && ENV["LOGSTASH_SOURCE"] == "1" | ||
| if Dir.exist?(logstash_path) && use_logstash_source |
There was a problem hiding this comment.
I don't think use_logstash_source is really needed, because of my previous comment.
There was a problem hiding this comment.
It's the same like in Beats input: https://github.com/logstash-plugins/logstash-input-beats/blob/main/Gemfile#L8
There was a problem hiding this comment.
While it may be, I still don't think it is a necessary change.
Example 1: In this code, you are setting use_logstash_source to 1 if LOGSTASH_SOURCE is equal to 1.
use_logstash_source = ENV['LOGSTASH_SOURCE'] && ENV['LOGSTASH_SOURCE'].to_s == '1'
if Dir.exist?(logstash_path) && use_logstash_source
Example 2: The above is the same as the following code, which is evaluating if LOGSTASH_SOURCE is equal to 1.
if Dir.exist?(logstash_path) && ENV["LOGSTASH_SOURCE"] == "1"
Meaning that the change is not really necessary.
There was a problem hiding this comment.
Yes you are right,
ENV['LOGSTASH_SOURCE'] && ENV['LOGSTASH_SOURCE'].to_s == '1'provides the same result as
ENV['LOGSTASH_SOURCE'] == '1'but my point here is to use the exact Gemfile that's used in all the other plugins. If we change here should we align also the others?
Gemfile
Outdated
| gem 'logstash-core', :path => "#{logstash_path}/logstash-core" | ||
| gem 'logstash-core-plugin-api', :path => "#{logstash_path}/logstash-core-plugin-api" | ||
| end | ||
|
|
||
| if RUBY_VERSION == "1.9.3" | ||
| gem 'rake', '12.2.1' | ||
| end | ||
|
|
There was a problem hiding this comment.
There's no reason to add 3 empty lines here.
Gemfile
Outdated
|
|
||
| if Dir.exist?(logstash_path) && ENV["LOGSTASH_SOURCE"] == "1" | ||
| if Dir.exist?(logstash_path) && use_logstash_source |
There was a problem hiding this comment.
While it may be, I still don't think it is a necessary change.
Example 1: In this code, you are setting use_logstash_source to 1 if LOGSTASH_SOURCE is equal to 1.
use_logstash_source = ENV['LOGSTASH_SOURCE'] && ENV['LOGSTASH_SOURCE'].to_s == '1'
if Dir.exist?(logstash_path) && use_logstash_source
Example 2: The above is the same as the following code, which is evaluating if LOGSTASH_SOURCE is equal to 1.
if Dir.exist?(logstash_path) && ENV["LOGSTASH_SOURCE"] == "1"
Meaning that the change is not really necessary.
Release notes
[rn:skip]
What does this PR do?
Fix the
Gemfileto properly set variables to use local Logstash cloneWhy is it important/What is the impact to the user?
Let the developer to use local clone, when configuring
LOGSTASH_PATHandLOGSTASH_SOURCEinstead of raisingjava.lang.NoClassDefFoundErrorChecklist
[ ] I have commented my code, particularly in hard-to-understand areas[ ] I have made corresponding changes to the documentation[ ] I have made corresponding change to the default configuration files (and/or docker env variables)[ ] I have added tests that prove my fix is effective or that my feature worksAuthor's Checklist
How to test this PR locally
Follow instruction in #81
Related issues
Use cases
Screenshots
Logs