Skip to content
This repository has been archived by the owner on Jul 4, 2023. It is now read-only.

logstash 1.4.0 #28598

Closed
wants to merge 3 commits into from
Closed

logstash 1.4.0 #28598

wants to merge 3 commits into from

Conversation

hawko2600
Copy link
Contributor

Upgrades logstash formula to 1.4.0.
Due to radical distribution changes upstream, we need to inreplace a few lines
to get it to run from the Cellar.

Upgrades logstash formula to 1.4.0.
Due to radical distribution changes upstream, we need to inreplace a few lines
to get it to run from the Cellar.
@adamv
Copy link
Contributor

adamv commented Apr 22, 2014

Error Message

failed: brew audit logstash
Stacktrace

        Error: 3 problems in 1 formulae
logstash:
 * HEAD: redundant :using specification in URL
 * Non-libraries were installed to "/usr/local/Cellar/logstash/1.4.0/lib".
Installing non-libraries to "lib" is discouraged.
The offending files are:
  /usr/local/Cellar/logstash/1.4.0/lib/logstash-event.rb
  /usr/local/Cellar/logstash/1.4.0/lib/logstash.rb
 * Non-executables were installed to "/usr/local/Cellar/logstash/1.4.0/bin".
The offending files are:
  /usr/local/Cellar/logstash/1.4.0/bin/logstash.bat
  /usr/local/Cellar/logstash/1.4.0/bin/logstash.lib.sh

@hawko2600
Copy link
Contributor Author

All the installed files are as per upstream. Esp. logstash.lib.sh is
required as its sourced by the main script. Whilst changing it is possible,
we would then be diverging from upstream & causing confusion with support
there.

I also disagree with the warning on the head URL, explicit is better than
implicit and a https schema implies no SCM tool.

How would you suggest these be addressed?

@adamv
Copy link
Contributor

adamv commented Apr 22, 2014

  • remove the :using per house style.
  • maintain an installation into libexec that wraps the appropriate binaries into bin.

Upgrade logstash to 1.4.0

* added test suite, will test a simple log flow
* added caveats with link to documentation for beginners
* passes brew audit

def install
libexec.install "logstash-#{version}-flatjar.jar"
bin.write_jar_script libexec/"logstash-#{version}-flatjar.jar", "logstash"
inreplace "bin/logstash", /^basedir=.*$/, "basedir=#{libexec}"
Copy link
Contributor

Choose a reason for hiding this comment

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

The first argument to inreplace can be a list:

inreplace %w{bin/logstash bin/logstash.lib.sh} ...

Let's use that since the replacements are the same in both of these files.

@adamv adamv self-assigned this Apr 24, 2014
* KNF
* remove EOF marker from second test to help build-bot parse it

Build from --HEAD breaks homebrew downloading conventions a lot,
so it was added then removed before commit.  Upstream logstash
needs work on their build system before we should consider it.
@hawko2600
Copy link
Contributor Author

Due to violating the rule about not downloading anything (about 50 times over), I opted against adding build from --head to this commit. Its risky anyway as it worked twice and failed three times during testing due to bad timing with upstream FTBFS errors. If anyone cares, add a head url to the github repo and "system make test" in the install block before the inreplace call (wrap in testing for build.head etc.) - that'll get you most of the way.

@FunTimeCoding
Copy link

what about just downloading their precompiled tarball? https://download.elasticsearch.org/logstash/logstash/logstash-1.4.0.tar.gz

they explained how to use it there: http://logstash.net/docs/1.4.0/release-notes

@adamv
Copy link
Contributor

adamv commented May 1, 2014

The precompiled package is acceptable, it's basically what the old flatjar was doing.

@FunTimeCoding
Copy link

sounds good. I just pondered why 1.3.3 was still in brew and tried updating it by simply replacing the url to 1.4.0 - but frankly that didn't exist. Im utterly new to writing formulae. could you update that or give me some pointers on how Id get there?

@FunTimeCoding
Copy link

https://gist.github.com/FunTimeCoding/620bdc43e14cb4f22ad7 Ive written a launchd plist which currently works with 1.3.3, I noticed other packages have some placeholders there, but it can be reused.

@adamv adamv removed their assignment May 1, 2014
@hawko2600
Copy link
Contributor Author

@FunTimeCoding
This uses the precompiled tarball. I didn't add the plist since application of logstash is entirely deployment-specific - you need a config file, you need to start whatever sub-service you want (agent, web, etc). Using a plist makes far too many assumptions about what the end user wants to do, also assumes the user only wants to run one instance (which is not always the case, e.g. my production deployment has two for different purposes (a shipper and an indexer - I run kibana and elasticsearch separately so only use agents).

But just because I don't think its useful, doesn't mean we shouldn't do it. If @adamv finds the last pull request acceptable than you may like to pull req your own plist on top of this... I just strongly recommend RunAtLoad be set false by default & extend the caveat section regarding letting the user change this after they write a config.

@FunTimeCoding
Copy link

I see. well you got a point that there may be various scenarios and topologies that can be built with Logstash, but isn't it also more straight forward to be presented with a generic configuration that can be adapted if so desired? a lower initial knowledge barrier is more likely to get people interested and digging deeper.

@hawko2600
Copy link
Contributor Author

That sounds like a good idea for a future improvement; we could e.g. index /var/log/*.log and setup the plist to run it on load; not conflicting on ports for the web interface would be much harder.

@adamv
Copy link
Contributor

adamv commented May 6, 2014

I think this is good now; any objections (from people in this thread who use it?)

Certainly future improvements are welcome.

@adamv adamv self-assigned this May 6, 2014
@apriendeau
Copy link
Contributor

👍 for this please.

@FunTimeCoding
Copy link

no objections. whats next?

@adamv adamv closed this in b2d2cf6 May 7, 2014
@Homebrew Homebrew locked and limited conversation to collaborators Feb 17, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants