Skip to content
This repository has been archived by the owner on Mar 4, 2024. It is now read-only.

LOG-8728 running saved query support on query commands #46

Merged
merged 1 commit into from
Aug 28, 2017

Conversation

stopal-r7
Copy link
Contributor

@stopal-r7 stopal-r7 commented Aug 24, 2017

This PR adds saved query support to various query related commands such as get events, get recentevents, query, tail events. This also extends validation of query commands.

There is some commit noise as I had to do some refactoring after bumping onto some code duplication on query module. Also let me know if you think there's a better way of refactoring this part of code.

@stopal-r7 stopal-r7 requested review from ilyabiryukov-r7 and removed request for scawley-r7 August 25, 2017 09:30
log_keys = kwargs.get('log_keys')
querynick = kwargs.get('querynick')
lognick = kwargs.get('lognick')
loggroup = kwargs.get('loggroup')
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this a logset? For naming consistency we should probably use the convention name

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is a known issue with the naming of "loggroup", "lognick" and "querynick" in lecli. They are not related to shortcuts, logsets etc we have in the server side. And they are confusing at some sense. There is actually an open bug for this already. We could solve it in another PR or this one. However, we don't have the names to go for the moment.

saved_query_id = kwargs.get('saved_query_id')
query_string = kwargs.get('query_string')
log_keys = kwargs.get('log_keys')
querynick = kwargs.get('querynick')
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here, regards naming. Should it be 'query name' instead?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants