Skip to content
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

[Metricbeat] Remove EventFetcher and EventsFetcher interface #11762

Closed
wants to merge 2 commits into from

Conversation

ruflin
Copy link
Member

@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
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
Member 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
Member 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
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

urso commented Apr 7, 2021

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

@exekias
Copy link
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
Member Author

ruflin commented Apr 8, 2021

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

@urso
Copy link

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
Member Author

ruflin commented Apr 8, 2021

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

@urso
Copy link

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
Member

Hi, yes, I will take a look 👍

@jsoriano
Copy link
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