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

React js #30

Merged
merged 5 commits into from
Nov 30, 2024
Merged

React js #30

merged 5 commits into from
Nov 30, 2024

Conversation

maurerle
Copy link
Collaborator

I barely have experience with react JS and the grafana sdk, but I somehow managed to make this work with the help of @intermittentnrg from #19

This is just a draft to provide some substance of fixing things :)

I don't know how one can work around loading the initial dashboard without losing the javascript context. maybe someone can help here.

Note that there is also this.grafanaRuntime.getDashboardTimeRange() and stuff like this.

@intermittentnrg
Copy link
Collaborator

Great work!

Would prefer it if it was split into multiple smaller PRs tho. I'll leave some comments.

Btw I saw you work with energy data, have you checked out my grafana website? I have data for ~170 areas and all code is open source on github.

@maurerle
Copy link
Collaborator Author

Hi @intermittentnrg this is not really meant as a PR which should get merged, as there are still various things to be fixed:

  • login does not work
  • changing the view of the dashboard dynamically does not work
  • hooking into events was not tested
  • I don't really know which parts of the code should be removed now
  • Without this 7f44ea3 - the resulting output has a far too high height, which should be fixed

@maurerle
Copy link
Collaborator Author

Regarding work on energy data, Thanks!

Unfortunately, your Grafana does not yet have the scenes api..?
So I can't make a video of this super nice map: https://intermittent.energy/d/fa529e06-ff34-415d-adf1-dde1a6f28350/prices-plotly-map?orgId=1&from=now-7d&to=now&viewPanel=2
Can you update it maybe? :)

@intermittentnrg
Copy link
Collaborator

intermittentnrg commented Nov 29, 2024

Awesome. Yes Timescale sponsored me with free Timescale cloud which I share with Frank Boerman.
The idea is that while our projects are completely separate we can at least query each others tables.

But I used US location cause it's slightly cheaper and I optimize cloud bills even when someone else pays the bill, and Frank Boerman insisted on EU location. I will migrate to same server eventually but it's likely at least a days work to copy all data and sync etc, so for now we only share cloud account, not instance.

Yes I need to update Grafana. Working on other things also. I've been posting price map videos by driving browser and filling in timepicker and posting to https://x.com/IntermittentNRG

Feel free to DM on X, I also have a energy grafana club group chat on X which it sounds like you should be a part of, tho it's mostly inactive. I spotted a bunch of instances using grafana for energy.

As for the PR I think some changes you made are great and should be merged. If you split them up I'm happy to merge, @amotl gave me permission to merge for this repo. Or I could split them myself?

@maurerle
Copy link
Collaborator Author

Oh cool, here it is the university institute hosting it on a VM with 64GB RAM and 48 vCPU for everything, running on timescaleDB as well 😊

Very cool! :)
I don't have an X Account though, but thank you!

If you have a little time, it would be great to check into the open issues from #30 (comment)

Feel free to split this up and adapt it, unfortunately I do not have that much knowledge of reactJS so I could need some help with this.
I mean, this project currently works well for grafana versions <11

After merging, it would only work with issues for Grafana >=11.3 - so we should at least get good support for latest Grafana :|

Copy link
Contributor

@amotl amotl left a comment

Choose a reason for hiding this comment

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

Thank you so much for this excellent refurbishment patch. 💯

This is not really meant as a PR which should get merged,

I think we should merge without much ado, because currently the program is broken anyway, right? I think there is no need to be backward-compatible with older releases of Grafana, or to create any other blockers that would hold back any progress on making this software work well reasonably again.

[...] as there are still various things to be fixed [...]

All flaws and caveats can be documented and tackled on behalf of subsequent patches, and I am looking forward to see them coming. 🍀

After merging, it would only work with issues for Grafana >=11.3 - so we should at least get good support for latest Grafana :|.

Please go ahead. 🙏

This was referenced Nov 29, 2024
@maurerle maurerle marked this pull request as ready for review November 29, 2024 20:49
@maurerle
Copy link
Collaborator Author

I am fine with merging this now.

@intermittentnrg
Copy link
Collaborator

Changes look good I have no nitpicks / comments to add! Of course there's more changes to make in further PRs.

Also can dashboard uid can be removed and just use the URL?
I never understood why I have to provide the uid in both the URL and as separate parameter.

@maurerle
Copy link
Collaborator Author

The dashboard uid has to be provided individually as there are two modes:
Dahsboard mode (with /d/ inbetween)
Panel mode (with /d-solo/ inbetween)

So thats why it is needed that way :)

@maurerle maurerle merged commit e2251cf into grafana-toolbox:main Nov 30, 2024
3 checks passed
@maurerle maurerle deleted the react_js branch November 30, 2024 10:05
@intermittentnrg
Copy link
Collaborator

Hmm OK, I just make custom copies of my dashboards for use with grafanimate so I never even noticed that there's a panel mode.

Good job!

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