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

[BUG] Telemetry plugin cluster info rename error #2043

Merged

Conversation

kavilla
Copy link
Member

@kavilla kavilla commented Aug 2, 2022

Description

Introduced when renaming, I accidentally typed the param
incorrectly.

Signed-off-by: Kawika Avilla kavilla414@gmail.com

Issues Resolved

n/a

Check List

  • New functionality includes testing.
    • All tests pass
      • yarn test:jest
      • yarn test:jest_integration
      • yarn test:ftr
  • New functionality has been documented.
  • Commits are signed per the DCO using --signoff

Introduced when renaming, I accidentally typed the param
incorrectly.

Issue:
n/a

Signed-off-by: Kawika Avilla <kavilla414@gmail.com>
@kavilla kavilla requested a review from a team as a code owner August 2, 2022 09:03
@kavilla kavilla added bug Something isn't working rename labels Aug 2, 2022
@codecov-commenter
Copy link

Codecov Report

Merging #2043 (8dfc29a) into main (54c2cdc) will increase coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #2043      +/-   ##
==========================================
+ Coverage   67.49%   67.51%   +0.01%     
==========================================
  Files        3076     3077       +1     
  Lines       59144    59184      +40     
  Branches     8989     9003      +14     
==========================================
+ Hits        39919    39957      +38     
- Misses      17041    17042       +1     
- Partials     2184     2185       +1     
Impacted Files Coverage Δ
...ry/server/telemetry_collection/get_cluster_info.ts 100.00% <100.00%> (ø)
...ore/public/chrome/ui/header/header_breadcrumbs.tsx 100.00% <0.00%> (ø)
...s/vis_type_vega/public/vega_view/vega_base_view.js 55.55% <0.00%> (ø)
packages/osd-std/src/validate_object.ts 91.30% <0.00%> (ø)
...c/plugins/vis_type_vega/public/data_model/utils.ts 73.33% <0.00%> (+34.87%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us.

Copy link
Member

@joshuarrrr joshuarrrr left a comment

Choose a reason for hiding this comment

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

Can you add a link to the PR or commit where the wrong name was introduced? Also, a note on how the bug was detected and fix validated would be useful in lieu of an issue. Do we need any additional test coverage or cases?

@manasvinibs
Copy link
Member

Can you add a link to the PR or commit where the wrong name was introduced? Also, a note on how the bug was detected and fix validated would be useful in lieu of an issue. Do we need any additional test coverage or cases?

Good call outs!

@kavilla kavilla merged commit 28fd642 into opensearch-project:main Aug 2, 2022
@kavilla kavilla deleted the avillk/rename_err_for_telemetry branch August 2, 2022 23:19
opensearch-trigger-bot bot pushed a commit that referenced this pull request Aug 2, 2022
Origin:
2e9d038#diff-78de4f9b624b3e4781283b5c5ab84428a1aa0920fb3ae86f60453d9d524922a7

Introduced when renaming, I accidentally typed the param
incorrectly.

Issue:
n/a

Signed-off-by: Kawika Avilla <kavilla414@gmail.com>
(cherry picked from commit 28fd642)
@kavilla
Copy link
Member Author

kavilla commented Aug 2, 2022

Can you add a link to the PR or commit where the wrong name was introduced? Also, a note on how the bug was detected and fix validated would be useful in lieu of an issue. Do we need any additional test coverage or cases?

I have updated the merge commit with the commit reference as requested: 2e9d038#diff-78de4f9b624b3e4781283b5c5ab84428a1aa0920fb3ae86f60453d9d524922a7.

Also I believe the problem is related to this: #1660, why it won't necessarily cause issues runtime but the TypeScript error was ignored in the compiler.

I discovered it by coincidentally unfortunately. Nothing specific.

ananzh pushed a commit that referenced this pull request Aug 3, 2022
Origin:
2e9d038#diff-78de4f9b624b3e4781283b5c5ab84428a1aa0920fb3ae86f60453d9d524922a7

Introduced when renaming, I accidentally typed the param
incorrectly.

Issue:
n/a

Signed-off-by: Kawika Avilla <kavilla414@gmail.com>
(cherry picked from commit 28fd642)

Co-authored-by: Kawika Avilla <kavilla414@gmail.com>
noCharger pushed a commit that referenced this pull request Aug 3, 2022
Origin:
2e9d038#diff-78de4f9b624b3e4781283b5c5ab84428a1aa0920fb3ae86f60453d9d524922a7

Introduced when renaming, I accidentally typed the param
incorrectly.

Issue:
n/a

Signed-off-by: Kawika Avilla <kavilla414@gmail.com>
CPTNB pushed a commit to CPTNB/OpenSearch-Dashboards that referenced this pull request Aug 8, 2022
…2043)

Origin:
opensearch-project@2e9d038#diff-78de4f9b624b3e4781283b5c5ab84428a1aa0920fb3ae86f60453d9d524922a7

Introduced when renaming, I accidentally typed the param
incorrectly.

Issue:
n/a

Signed-off-by: Kawika Avilla <kavilla414@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants