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

Adding support to preserve annotations at stream and app level. #39

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Dr0na
Copy link
Contributor

@Dr0na Dr0na commented May 24, 2019

Currently, the query is simply appended to stream definitions. If the query has annotations then the query becomes invalid. Hence the fix. If someone wants to enable statistics for a query, s/he has to add an app level annotation @app:statistics(reporter = 'console') before the query. However, this is currently not supported as the query is directly appended to the stream definitions misplacing the annotation and renders the query invalid. The fix is to insert the stream definitions just before the query. Below is the sample siddhi query with annotations which can be used for testing the fix.

@app:statistics(reporter = 'console')
@query(name = 'echo')
from inputStream
select *
insert into outputStream

Currently, the query is simply appended to stream definitions. If the query has annotations then the query becomes invalid. Hence the fix. If someone wants to enable statistics for a query, s/he has to add an app level annotation @app:statistics(reporter = 'console') before the query. However, this is currently not supported as the query is directly appended to the stream definitions misplacing the annotation and renders the query invalid. The fix is to insert the stream definitions just before the query. Below is the sample siddhi query with annotations which can be used for testing the fix.

@app:statistics(reporter = 'console') 
@query(name = 'echo')
from inputStream 
 select * 
 insert into outputStream
@haoch
Copy link
Owner

haoch commented Jun 27, 2019

@Dr0na
Thanks for your contribution!

A few comments:

  1. Could you please add some test for the change?
  2. Could you use SiddhiParser to parse the executionPlan to get the annotation of @app and @info instead of parsing string directly?

@haoch
Copy link
Owner

haoch commented Sep 16, 2019

@Dr0na How do you think about my comments above?

@haoch haoch added the review label Oct 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants