-
-
Notifications
You must be signed in to change notification settings - Fork 104
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
update filter search #396
update filter search #396
Conversation
Now it's possible to search multiple values of the same type.
Pull Request Test Coverage Report for Build 1385
💛 - Coveralls |
@afeena what should we do about 3.6 tests failing? |
applied_filters['end_time'] = applied_filters['end_time'] | ||
key, value = filt.split(':', 1) | ||
applied_filters[key].append(value) | ||
print(applied_filters) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove print
where = "S.sensor_id='%s'" % (filters["sensor_id"][0]) | ||
|
||
for parameter, values in filters.items(): | ||
if parameter == "attack_type": |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought a lot about this query building, can we rebuilt it in a way it looks more accurate and we use the power of sqlalchemy? like here https://aiopg.readthedocs.io/en/stable/examples.html#complex-sqlalchemy-queries
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I actually thought about it and I think almost everything would stay similar to the way it is.
The only thing that would differ is that instead of having raw queries we would be using the sqlalchemy objects
I mean all these conditions would have to stay so code might not become as clean as we think.
But I can definitely try to do it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could keep it in mind and make as a separate task :)
@mzfr I think we should remove python 3.6 from travis config, what do you think? |
Now it's possible to search multiple values of the same type.
When I initially made change to the API it only supported filter search in the following way:
but failed to support something like:
This PR add that support