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

fix hidden_queries plan bug #201

Merged
merged 3 commits into from
Jan 11, 2024
Merged

fix hidden_queries plan bug #201

merged 3 commits into from
Jan 11, 2024

Conversation

matthagenbuch
Copy link
Contributor

Fixes a bug when using the lightstep_alert.query.hidden_queries attribute. Currently, if this attribute is set the plan will always show a change because it isn't being read during the refresh stage. This change reads the hidden_queries into the resource during refresh, resulting in a clean plan.

@matthagenbuch matthagenbuch requested review from MisterSquishy, lambcode and a team January 11, 2024 16:26
Copy link
Contributor

@MisterSquishy MisterSquishy left a comment

Choose a reason for hiding this comment

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

weirdddd how was this test working without a ExpectNonEmptyPlan? did we add handling for this in dashboards but forget about alerts?

either way maybe worth an analogous test in alerts!

@MisterSquishy
Copy link
Contributor

oh also dont forget to update .go-version!

Copy link
Contributor

@mattcarmody mattcarmody left a comment

Choose a reason for hiding this comment

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

LGTM

@matthagenbuch matthagenbuch merged commit 8a92fc0 into main Jan 11, 2024
5 checks passed
@matthagenbuch matthagenbuch deleted the matth.fix_hidden_queries branch January 11, 2024 16:49
matthagenbuch added a commit that referenced this pull request Jan 17, 2024
matthagenbuch added a commit that referenced this pull request Jan 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants