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

Feature/sql logic #186

Merged
merged 19 commits into from
Jul 24, 2020
Merged

Feature/sql logic #186

merged 19 commits into from
Jul 24, 2020

Conversation

fgbogdan
Copy link
Contributor

@fgbogdan fgbogdan commented May 14, 2020

Hi,
we use your plugin in our app - and we have added some logic in CH sql before sending it to CH.
The sqlquery is processed before to be sent to CH, using some minimal javascript conditional syntax as:

<%if(($to - $from) / 3600 > 3){%> 
   SELECT something FROM ...
<%}else{%>
  SELECT anotherThing FROM ...
<%}%>

The available keywords are:
var|if|for|else|switch|case|break

Please let me know if you need more details or samples.
Thanks for your work till now !

@Slach
Copy link
Collaborator

Slach commented Jun 15, 2020

Hello @fgbogdan, sorry for late resonse,
could you resolve conflicts
and add documentation to README with examples
and also add jest unit tests?

@fgbogdan
Copy link
Contributor Author

fgbogdan commented Jun 16, 2020

Hi, Thanks for your answer.
I have added details in README.MD.
The conflicts where generated by dist modules - and these are generated on each compile. (npm build).
Tests - will try.

src/scanner.ts Show resolved Hide resolved
@Slach
Copy link
Collaborator

Slach commented Jun 16, 2020

@fgbogdan look test cases on https://github.com/Vertamedia/clickhouse-grafana/blob/master/spec/scanner_specs.jest.ts and add test cases for your render() function

@fgbogdan
Copy link
Contributor Author

Hi again,

I have added the test for matching <%.%> and also some tests in scanner_specs.jest.ts
For the escape mechanism - will think about.

@fgbogdan fgbogdan closed this Jun 16, 2020
@fgbogdan fgbogdan reopened this Jun 16, 2020
@fgbogdan
Copy link
Contributor Author

Hi Slach,
should I do something else to fulfill the case ?
Still no idea for "escape mechanism" when the tables or fields contains themselves <% or %>, excepting some dummy escape chars like /<% or <<%.
Any idea will be appreciate.
10x
Bogdan

@Slach Slach closed this Jul 21, 2020
@Slach Slach reopened this Jul 22, 2020
Copy link
Collaborator

@Slach Slach left a comment

Choose a reason for hiding this comment

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

@fgbogdan could you run docker-compose run frontend_builder and docker-compose run backend_builder and check render function works with alert functionality?

could you add some example dashboards with SQL logic and alerts to docker/grafana/dashboards/?

@fgbogdan
Copy link
Contributor Author

Hi, ok - will try that next week.

@Slach
Copy link
Collaborator

Slach commented Jul 23, 2020

will wait, thanks a lot for contributing

@fgbogdan
Copy link
Contributor Author

Hi, got some time this morning and added the dash with sql logic and alert. If you want something more fancy just let me know

@Slach Slach merged commit 0c6efab into Altinity:master Jul 24, 2020
@Slach
Copy link
Collaborator

Slach commented Jul 24, 2020

@fgbogdan thanks for contributing

@Slach Slach mentioned this pull request Jul 24, 2020
Slach added a commit that referenced this pull request Jul 24, 2020
# 2.0.3 (2020-07-24)

## Enhancements:
* add setup notes for Grafana 7.x to README
* add SQL preprocessing logic on browser side with <% js code subset %>, #186, thanks @fgbogdan
* improve alerts query processing for use case when `query(query_name, from, to)` time range is less than visible dashboard time range, see #237
* improve alerts json parsing in golang part for case when we have string fields in response which interprets as series name, see #230
* properly parsing POST queries in golang part of plugin, #228, thanks @it1804


## Fixes:
* add Vagrantfile for statefull environment and allow to upgrade scenario like  grafana 7.1.0 + grafana-cli upgrade-all
  * fix #244
  * fix #243
* add multiple dashboard examples for github issues:
  * fix #240 
  * fix #135 
  * fix #245 
  * fix #238   
  * fix #232
  * fix #127
  * fix #141
Slach added a commit to Altinity/grafana-plugin-repository that referenced this pull request Jul 24, 2020
## Enhancements:
* add setup notes for Grafana 7.x to README
* add SQL preprocessing logic on browser side with <% js code subset %>, Altinity/clickhouse-grafana#186, thanks @fgbogdan
* improve alerts query processing for use case when `query(query_name, from, to)` time range is less than visible dashboard time range, see Altinity/clickhouse-grafana#237
* improve alerts json parsing in golang part for case when we have string fields in response which interprets as series name, see Altinity/clickhouse-grafana#230
* properly parsing POST queries in golang part of plugin, Altinity/clickhouse-grafana#228, thanks @it1804

## Fixes:
* add Vagrantfile for statefull environment and allow to upgrade scenario like  grafana 7.1.0 + grafana-cli upgrade-all
  * fix Altinity/clickhouse-grafana#244
  * fix Altinity/clickhouse-grafana#243
* add multiple dashboard examples for github issues:
  * fix Altinity/clickhouse-grafana#240
  * fix Altinity/clickhouse-grafana#135
  * fix Altinity/clickhouse-grafana#245
  * fix Altinity/clickhouse-grafana#238
  * fix Altinity/clickhouse-grafana#232
  * fix Altinity/clickhouse-grafana#127
  * fix Altinity/clickhouse-grafana#141

Signed-off-by: Eugene Klimov <[email protected]>
@fgbogdan fgbogdan deleted the feature/SQLLogic branch July 24, 2020 15:15
@Slach
Copy link
Collaborator

Slach commented Aug 5, 2020

sorry @fgbogdan this functionality make this plugin to available XSS attacks, I need revert it ASAP

Slach added a commit to Altinity/grafana-plugin-repository that referenced this pull request Aug 13, 2020
# 2.1.0 (2020-08-13)

## Enhancement:
* add "Skip comments" checkbox to query editor to pass SQL comments to server, fix Altinity/clickhouse-grafana#265
* add setup notes for Grafana 7.x to README
* add SQL preprocessing logic on browser side with <% js code subset %>, Altinity/clickhouse-grafana#186, thanks @fgbogdan
* improve alerts query processing for use case when `query(query_name, from, to)` time range is less than visible dashboard time range, see Altinity/clickhouse-grafana#237
* improve alerts json parsing in golang part for case when we have string fields in response which interprets as series name, see Altinity/clickhouse-grafana#230
* properly parsing POST queries in golang part of plugin, Altinity/clickhouse-grafana#228, thanks @it1804

## Fixes:
* fix corner cases for $macro + subquery, see Altinity/clickhouse-grafana#276 and Altinity/clickhouse-grafana#277
* fix parallel query execution, see Altinity/clickhouse-grafana#273
* fix identifiers quotes, see Altinity/clickhouse-grafana#276, Altinity/clickhouse-grafana#277
* fix plugin.json for pass `grafana-plugin-repository` plugin validator
* fix multi-value variables behavior - Altinity/clickhouse-grafana#252
* add Vagrantfile for statefull environment and allow to upgrade scenario like  grafana 7.1.0 + grafana-cli upgrade-all
  * fix Altinity/clickhouse-grafana#244
  * fix Altinity/clickhouse-grafana#243
* add multiple dashboard examples for github issues:
  * fix Altinity/clickhouse-grafana#240
  * fix Altinity/clickhouse-grafana#135
  * fix Altinity/clickhouse-grafana#245
  * fix Altinity/clickhouse-grafana#238
  * fix Altinity/clickhouse-grafana#232
  * fix Altinity/clickhouse-grafana#127
  * fix Altinity/clickhouse-grafana#141

Signed-off-by: Eugene Klimov <[email protected]>
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.

2 participants