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] Migrate Couchbase bucket to use ReporterV2 interface #11045

Merged

Conversation

berfinsari
Copy link
Contributor

@berfinsari berfinsari commented Mar 3, 2019

Refer to #10774 for more info

@berfinsari berfinsari requested a review from a team as a code owner March 3, 2019 01:58
@elasticmachine
Copy link
Collaborator

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

@sayden sayden added Metricbeat Metricbeat Team:Integrations Label for the Integrations team labels Mar 4, 2019
@sayden
Copy link
Contributor

sayden commented Mar 4, 2019

jenkins, test this please

Copy link
Contributor

@sayden sayden left a comment

Choose a reason for hiding this comment

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

Overall looks good. Good work! Just one small change

assert.NotEmpty(t, events)
t.Logf("%s/%s event: %+v", f.Module().Name(), f.Name(), events[0])
}

func TestData(t *testing.T) {
Copy link
Contributor

@sayden sayden Mar 7, 2019

Choose a reason for hiding this comment

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

As commented in other PR's, to avoid potential flaky tests TestData must now look like this:

func TestData(t *testing.T) {
	f := mbtest.NewReportingMetricSetV2(t, getConfig())

	if err := mbtest.WriteEventsReporterV2(f, t, ""); err != nil {
		t.Fatal("write", err)
	}
}

@sayden
Copy link
Contributor

sayden commented Mar 7, 2019

Changed my mind, it's enough with this, we can do a follow up PR, this module doesn't fails 😄 Thanks @berfinsari !

@sayden sayden merged commit 35af9fb into elastic:master Mar 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Metricbeat Metricbeat Team:Integrations Label for the Integrations team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants