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

Merge changes from hiveeyes and use toolkit #214

Open
wants to merge 92 commits into
base: master
Choose a base branch
from

Conversation

ryantxu
Copy link
Member

@ryantxu ryantxu commented Aug 29, 2019

This is a PR from my toolkit work on hiveeyes. To actually merge it we should:

  • fix the plugin id
  • set the version to 0.3
  • remove the custom build tooling
  • test it well

maybe first commit a change to worldmap that removes dist so the PR is not so large

amotl and others added 30 commits April 20, 2019 00:19
location information from JSON endpoint, even for table data.
The lookup key is the value from the obtained "labelField".
- locationData: “table+json(p)”
- showZoomControl
- showAttribution
- legendContainerSelector
This is a simple implementation of a panel option being used to
navigate to a new URL on click.
…a#190

Add basic variable interpolation based on keys from `dataPoint`.
Saying that, all metric values have been added to the dataPoint
beforehand by prefixing them with `point_` to allow for setting
things like `clickthroughURL: /url/to/$point_station_id` if
there’s a field called `station_id` in the current datapoint.
into clickthrough links, thus interpolating dashboard **and** dataPoint 
variables now.
This enables the interpolation of dashboard variables into each 
of the panel control settings. An example would be to have a
variable called “countrycode” which gets populated by a 
database query defined by an SQL statement like:

SELECT
  country_code AS __value,
  country_name AS __text
FROM
  stations
ORDER BY
  country_code

Then, assigning a value like that to the JSON endpoint URL panel
control setting, the machinery would interpolate the selected
value assigned to the $countrycode variable appropriately:

  jsonUrl=/api/json/stations.json?country=$countrycode
interpolation dictionary, prefixed by `request_`.

This enables to use request variables in all panel control options. 
So, when invoking the dashboard with an url query parameter like 
`map_center_latitude=42.42`, you would be able to interpolate it to 
a panel control options by i.e. assigning 
`mapCenterLatitude: $request_map_center_latitude`.
to open clickthrough target in designated window.
Both options provide a more convenient way to determine the dimensions 
of the map section/frame. Both features are using Leaflet’s `getBounds()`
method. `mapFitData` will choose an optimal center and zoom level for
covering the displayed data while `mapZoomByRadius` will use the
bounding box of a respective circle with the designated radius
around the chosen center.
@ryantxu ryantxu mentioned this pull request Aug 29, 2019
@torkelo
Copy link
Member

torkelo commented Aug 29, 2019

It's a bit scary to merge this when it comes with so many new features and options that we have to support going forward, would require more testing as well to see if this breaks any existing dashboards.

But most importantly it does not fix the issue for me.
image

Would be nice to isolate the upgrade to grafana toolkit

@ryantxu
Copy link
Member Author

ryantxu commented Aug 29, 2019

It's a bit scary to merge this

Ya -- it is essentially the fork that has been getting more constant attention for the last year while the core plugin has not moved.

Merging toolkit changes first is hard since then most things are formatting issues :(

My main thought/concern about the work in worldmap-panel-ng is that most of it is adding better support for table data. This should all be moot with DataFrames, but either approach merging or converting to DataFrames will require some love and attention

@amotl
Copy link

amotl commented Aug 29, 2019

Dear @torkelo and @ryantxu,

thanks for giving our fork some attention. We very much appreciate the care you are giving to it regarding toolkit support and probably more, @ryantxu.

We see the change against upstream is pretty large already and we would also like to give this some more intense testing. Especially @smargo171's input on grafana-toolbox#10 would be worth to follow up on before considering going mainline.

Adding better support for table data should all be moot with DataFrames, but either approach merging or converting to DataFrames will require some love and attention.

While currently a bit swamped which prohibits immediate diving into the topic and codebase again, I would love to hear more about the DataFrames you mentioned here. Is it a new internal data carrier essentially making the wrestling with table format vs. timeseries format things obsolete at all?

Depending on which time frame you are looking at, I might be able to pick up our work on Worldmap NG, give it some more love and check its robustness on some edges.

Unfortunately, we haven't been able to dedicate ourselves to #213, so thanks a bunch for fixing that!

Please let me know / try to apply some pressure if you see that we can support you in the process of bringing the code base back to mainline again.

With kind regards,
Andreas.

@CLAassistant
Copy link

CLAassistant commented Aug 10, 2021

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
0 out of 6 committers have signed the CLA.

❌ ryft
❌ leonhardhaas
❌ fabienpomerol
❌ ryantxu
❌ amotl
❌ dtheb
You have signed the CLA already but the status is still pending? Let us recheck it.

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.

None yet

8 participants