grafana / sqlds

A package that assists writing SQL-driven datasources
Apache License 2.0
20 stars 12 forks source link

Add some default macros #45

Closed andresmgot closed 3 years ago

andresmgot commented 3 years ago

Fixes #43

I have added default macros and a suggested implementation (that should work for any SQL server). It's expected from the data sources to re-implement them if necessary, to adjust to their needs.

The goal is to set a common ground so all the SQL datasources implement at least a common set of macros following the pattern defined here.

andresmgot commented 3 years ago

Maybe it's worth continuing the discussion. I'm skeptical that any kind of default macro is going to work for every database.

I wonder if all databases even support comparing times with > or <.

Perhaps for those that don't, they could overwrite the key in the DefaultMacros map? If that's a possible use case we should document that :)

Yes, my intention here is to add macros that work in most databases (for example the > or < operand is valid for all the databases that use sqlds at the moment) but they can be overwritten by the datasources (and that would be the most common case for macros like $__timeGroup). In the tests there is an example of how to override a macro, they just need to re-define it:

https://github.com/grafana/sqlds/pull/45/files#diff-f12dec668f034baf10152ef12db851fe27e9b65b8b5a515ec08359027326c648R29

andresmgot commented 3 years ago

LGTM. Can we add few lines in README about this? otherwise will become difficult to track these changes in future.

Good point. I will add that.

andresmgot commented 3 years ago

Drone keeps not building my PRs :/ checked the tests locally and it passes so merging this to unblock me.