Skip to content

[Metricbeat] Remove EventFetcher and EventsFetcher interface#11762

Closed
ruflin wants to merge 2 commits intoelastic:masterfrom
ruflin:remove-event-fetcher
Closed

[Metricbeat] Remove EventFetcher and EventsFetcher interface#11762
ruflin wants to merge 2 commits intoelastic:masterfrom
ruflin:remove-event-fetcher

Conversation

@ruflin
Copy link
Copy Markdown
Contributor

@ruflin ruflin commented Apr 11, 2019

After migrating all metricsets (#10774) to the Reporter interfaces, the EventFetcher and EventsFetcher interface can be removed.

After migrating all metricsets (elastic#10774) to the Reporter interfaces, the EventFetcher and EventsFetcher interface can be removed.
@ruflin ruflin added in progress Pull request is currently in progress. Metricbeat Metricbeat technical_debt labels Apr 11, 2019
@ruflin ruflin requested a review from a team as a code owner April 11, 2019 11:51
@ruflin ruflin self-assigned this Apr 11, 2019
@exekias
Copy link
Copy Markdown
Contributor

exekias commented Apr 12, 2019

what would happen to existing community beats that upgrade libbeat? Not sure how frequently that happens

@ruflin
Copy link
Copy Markdown
Contributor Author

ruflin commented Apr 15, 2019

@exekias They will break and have to migrate to the new reporter. But in most cases this should be pretty straight forward and I don't think we should wait until 8 to remove this.

@ruflin ruflin added the Team:Integrations Label for the Integrations team label Apr 30, 2019
@ruflin ruflin assigned exekias and unassigned ruflin Apr 30, 2019
@ruflin
Copy link
Copy Markdown
Contributor Author

ruflin commented Apr 30, 2019

@exekias Assigning this to you for now, hope that is ok.

@zube zube bot removed the technical_debt label May 6, 2020
@mergify
Copy link
Copy Markdown
Contributor

mergify bot commented Apr 7, 2021

This pull request is now in conflicts. Could you fix it? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

git fetch upstream
git checkout -b remove-event-fetcher upstream/remove-event-fetcher
git merge upstream/master
git push upstream remove-event-fetcher

@urso
Copy link
Copy Markdown

urso commented Apr 7, 2021

@ruflin @exekias Have these interfaces already been removed, or shall we resurrect the PR?

@exekias
Copy link
Copy Markdown
Contributor

exekias commented Apr 8, 2021

I don't think this happened in the end, IIRC there were a few bits still pending in the PR

@ruflin
Copy link
Copy Markdown
Contributor Author

ruflin commented Apr 8, 2021

Should I best close this PR so someone can follow up with this?

@urso
Copy link
Copy Markdown

urso commented Apr 8, 2021

I'd love to remove that much code (when working on collector POC these interfaces did contribute to most complexity/code...). If there is something pending lets close and create an issue, otherwise get rid of it now.

@ruflin
Copy link
Copy Markdown
Contributor Author

ruflin commented Apr 8, 2021

@urso ++. Can someone from the team take this over?

@urso
Copy link
Copy Markdown

urso commented Apr 12, 2021

Not sure what is missing + I would like someone who is familiar with metricset implementation to change these kind of internals. @jsoriano Can you have a look?

@jsoriano
Copy link
Copy Markdown
Member

Hi, yes, I will take a look 👍

@jsoriano
Copy link
Copy Markdown
Member

Continuing with this in #25093.

@jsoriano jsoriano closed this Apr 14, 2021
@zube zube bot removed the [zube]: Backlog label Apr 14, 2021
@zube zube bot added the [zube]: Done label Apr 14, 2021
@zube zube bot removed the [zube]: Done label Jul 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

in progress Pull request is currently in progress. Metricbeat Metricbeat Team:Integrations Label for the Integrations team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants