logstash-plugins / logstash-filter-grok

Grok plugin to parse unstructured (log) data into something structured.
https://www.elastic.co/guide/en/logstash/current/plugins-filters-grok.html
Apache License 2.0
122 stars 97 forks source link

[wip] Suggested improvement/change to .travis.yml #139

Closed jsvd closed 4 years ago

jsvd commented 6 years ago

This is a suggested change to .travis.yml that, if agreed upon, should likely be mass pushed to all plugins

jakelandis commented 6 years ago

I have a similar change over at https://github.com/logstash-plugins/logstash-filter-elasticsearch/pull/95/commits/cf50bc15855e7adca953274387c2adac8628cd7a but using docker / docker compose that may worth discussing before pulling the trigger here.

webmat commented 6 years ago

I would love to get these speed improvements sooner than later.

Seems to me like moving to Docker for Travis & local dev will be a great improvement. But it sounds like it may take us a while to actually move to that. There's much more to understand, review and tweak. Would you say that's a fair characterization, @jakelandis? Or is your PR essentially ready to be reviewed as well?

If the worry is that we'll have to update all plugins twice rather than once, I can understand that. But depending on when we're ready for Docker, I wouldn't mind doing both updates, if it means getting a massive speed improvement a few weeks earlier.

Another thing, I think we should standardize the list of versions we test for, and consider allowing failures on some of the snapshots.

The idea is to minimize times like last week, where everything is broken for us because of active development on core.

Actually the above would not have prevented last week really. Perhaps we should still have branch testing (allowing failures), so we can warn core the same day, if something starts to fail?

jsvd commented 6 years ago

I don't think we need to rush this, this pr was an exercise in minimalism, and it's mostly useful for spec suites that have only unit testing. To be massively applied to all plugins it needs some extra work to accommodate integration test setups and teardowns. Also, I'd like to find a way to dynamically fetch the latest version for each major and state only:

LOGSTASH_MAJOR_VERSION=5 LOGSTASH_MAJOR_VERSION=6

Otherwise we'll be mass updating 200 .travis.yml files for each release of the stack.

Let's discuss a way forward with https://github.com/logstash-plugins/logstash-filter-elasticsearch/pull/95, namely see what's missing to have it be applied to all plugins.

webmat commented 6 years ago

Until Infra gives us an API that could help our scripts discover the most recent releases & dev versions per major version, we could use something somewhat manual like this:

https://github.com/logstash-plugins/plugins_ci_shared. Check out the readme for 2 examples.

After moving to an approach like this, we could trivially change the release and dev versions all plugin tests against by changing this the files in this repository's variables/* files.

jakelandis commented 6 years ago

I created a competing PR at https://github.com/logstash-plugins/logstash-filter-grok/pull/141 for comparison.

I have a slight preference to the docker approach, mostly because i like encapsulation of running locally and it matches an integration testing setup... so running unit vs. integration tests carry the same requirements of only having Docker installed.

kares commented 4 years ago

this is expected to be defunct now with https://github.com/logstash-plugins/logstash-filter-grok/pull/141 and more changes towards Docker-ized setup