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

Remove sceneObject from scoped vars #697

Closed
wants to merge 2 commits into from

Conversation

bfmatei
Copy link
Contributor

@bfmatei bfmatei commented Apr 16, 2024

Remove __sceneObject from scopedVars. This is causing various issues with datasources that are stringifing the request object for various reasons.

📦 Published PR as canary version: 4.9.1--canary.697.8706687886.0

✨ Test out this PR locally via:

npm install @grafana/scenes@4.9.1--canary.697.8706687886.0
# or 
yarn add @grafana/scenes@4.9.1--canary.697.8706687886.0

@bfmatei bfmatei added patch Increment the patch version when merged release Create a release when this pr is merged labels Apr 16, 2024
@bfmatei bfmatei requested review from torkelo and dprokop April 16, 2024 13:33
@bfmatei bfmatei self-assigned this Apr 16, 2024
Copy link
Member

@torkelo torkelo left a comment

Choose a reason for hiding this comment

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

@bfmatei this would break scenes compatability with legacy template system (templateSrv).

We rely on this scene object reference in calls to templateSrv.replace

https://github.com/grafana/grafana/blob/main/public/app/features/templating/template_srv.ts#L251

Many data sources pass scopedVars to templateSrv.replace and this is the way we know the scene object context so we can lookup the right scene scoped vars.

https://github.com/grafana/grafana/blob/main/packages/grafana-prometheus/src/variables.ts#L44

https://github.com/grafana/grafana/blob/main/packages/grafana-runtime/src/utils/DataSourceWithBackend.ts#L154

countless others

@dprokop
Copy link
Member

dprokop commented Apr 17, 2024

@torkelo yeah, you are totally. right, I've missed the fact that query method can call templateSrv.replace, damn.

We are trying to solve an issue with circular JSON serialisation in Zabbix plugin (they are building a string hash from the DataQueryRequest object by using JSON.stringify). We can of course fix it in Zabbix, but it feels like some other data sources may do a similar thing.

But anyways, I am afraid in the end it's the Zabbix where we need to provide the fix for this issue. Sorry @bfmatei for confusion!

@bfmatei
Copy link
Contributor Author

bfmatei commented Apr 17, 2024

Closing this in favor of alternative solution

@bfmatei bfmatei closed this Apr 17, 2024
@bfmatei bfmatei deleted the bogdan/remove-scene-object-scoped-vars branch April 17, 2024 08:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
patch Increment the patch version when merged release Create a release when this pr is merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants