-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
MINOR: mstr api response fix #18673
MINOR: mstr api response fix #18673
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One non-blocking comment
entity_type=Chart, | ||
service_name=self.context.get().dashboard_service, | ||
chart_name=chart, | ||
if dashboard_details: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What are the cases for this variable to not exist? Is it expected? Maybe better to log it so that we debug ingestions that "succeed but do nothing".
closing this. opened up a new PR here with the fix. Thanks |
Describe your changes:
Ports #18665. I don't have permissions on that fork and had to make some changes
I worked on ... because ...
Type of change:
Checklist:
Fixes <issue-number>: <short explanation>