Skip to content

Modify HTTP helper to not require a base Metricset#22573

Merged
sayden merged 5 commits intoelastic:masterfrom
sayden:modify_http_helper
Nov 18, 2020
Merged

Modify HTTP helper to not require a base Metricset#22573
sayden merged 5 commits intoelastic:masterfrom
sayden:modify_http_helper

Conversation

@sayden
Copy link
Copy Markdown
Contributor

@sayden sayden commented Nov 12, 2020

An example of how to modify the HTTP helper to not require a base metricset and use it as a dependency while testing

(cherry picked from commit 2ba6ec153603a4473d582bc0a140d66b3956ce72)
…TP Helper

(cherry picked from commit c8c0f6ff32454143ed1af62202736f544c981f02)

# Conflicts:
#	metricbeat/module/elasticsearch/cluster_stats/cluster_stats.go
#	metricbeat/module/elasticsearch/cluster_stats/data_test.go
@sayden sayden added the Metricbeat Metricbeat label Nov 12, 2020
@sayden sayden requested a review from ycombinator November 12, 2020 20:09
@sayden sayden self-assigned this Nov 12, 2020
@botelastic botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Nov 12, 2020
@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Nov 12, 2020

🐛 Flaky test report

❕ There are test failures but not known flaky tests.

Expand to view the summary

Test stats 🧪

Test Results
Failed 1
Passed 14
Skipped 0
Total 15

Genuine test errors 1

💔 There are test failures but not known flaky tests, most likely a genuine test failure.

  • Name: [build failed] – elasticsearch

@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Nov 12, 2020

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview

Expand to view the summary

Build stats

  • Build Cause: [sayden commented: jenkins run the tests, please]

  • Start Time: 2020-11-17T21:31:59.045+0000

  • Duration: 74 min 25 sec

Test stats 🧪

Test Results
Failed 0
Passed 2780
Skipped 629
Total 3409

💚 Flaky test report

Tests succeeded.

Expand to view the summary

Test stats 🧪

Test Results
Failed 0
Passed 2780
Skipped 629
Total 3409

Copy link
Copy Markdown
Contributor

@ycombinator ycombinator left a comment

Choose a reason for hiding this comment

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

LGTM.

Odd that newHTTPFromConfig used to take name as an argument but not use it.. Wonder why it did that. Anyway, looks safe to remove that argument now and ++ to exporting the function as well so we can use it from tests. 👍

@andresrc andresrc added the Team:Services (Deprecated) Label for the former Integrations-Services team label Nov 14, 2020
@botelastic botelastic bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Nov 14, 2020
@sayden sayden changed the title [Draft] Modify HTTP helper [Draft] Modify HTTP helper to be able to use it outside of Metricbeat Nov 17, 2020
@sayden sayden marked this pull request as ready for review November 17, 2020 13:23
@elasticmachine
Copy link
Copy Markdown
Contributor

Pinging @elastic/stack-monitoring (Stack monitoring)

@elasticmachine
Copy link
Copy Markdown
Contributor

Pinging @elastic/integrations-services (Team:Services)

@sayden
Copy link
Copy Markdown
Contributor Author

sayden commented Nov 17, 2020

jenkins run the tests, please

@sayden sayden changed the title [Draft] Modify HTTP helper to be able to use it outside of Metricbeat Modify HTTP helper to be able to use it outside of Metricbeat Nov 18, 2020
@sayden sayden merged commit fe8d7b3 into elastic:master Nov 18, 2020
@sayden sayden changed the title Modify HTTP helper to be able to use it outside of Metricbeat Modify HTTP helper to not require a base Metricset Nov 18, 2020
@sayden sayden deleted the modify_http_helper branch August 25, 2022 12:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Feature:Stack Monitoring Metricbeat Metricbeat Team:Services (Deprecated) Label for the former Integrations-Services team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants