-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Remove ID from Runner interface #5158
Conversation
Can one of the admins verify this patch? |
ok to test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, I think we should get rid of https://github.com/elastic/beats/blob/master/metricbeat/mb/module/wrapper.go#L40 and https://github.com/elastic/beats/blob/master/metricbeat/mb/module/wrapper.go#L132-L148
Anything against this @ruflin ?
@exekias SGTM |
👍 @vjsamuel can you remove that mentioned method? then this is good to go |
388668a
to
8c21d06
Compare
jenkins test it please |
We get this error at https://beats-ci.elastic.co/job/elastic+beats+pull-request+multijob-linux/1506/beat=filebeat,label=ubuntu/console:
|
8c21d06
to
28f2d9c
Compare
jenkins test it please |
I've reproduced the issue locally, although it doesn't make sense to me, it's gone on a second run 😕 |
28f2d9c
to
502e56d
Compare
Only checked travis and I see the following failure:
@exekias Is this related what you saw locally (once)? |
No, tests are OK, the only weird thing is around |
We decided to merge this as the |
Fixes #5153