-
Notifications
You must be signed in to change notification settings - Fork 68
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
ES aggregations #601
base: master
Are you sure you want to change the base?
ES aggregations #601
Conversation
wow, good stuff! looking forward |
ecd4e1d
to
96c9d0b
Compare
if (collectionScope._indexConfiguration.countUpdateIntervalMs) { | ||
intervalID = Meteor.setInterval( | ||
() => this.changed( | ||
collectionName, | ||
'searchCount' + definitionString, | ||
{ count: cursor.mongoCursor.count() } | ||
// TODO: I substituted cursor.mongoCursor.count with cursor.count. will this break things? | ||
{ count: cursor.count && cursor.count() || 0 } |
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.
@matteodem Will this still work for mongo?
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 think you'd have to check out all useages of the core Cursor
and check if it's simply the mongoCursor.count()
that's passed in. I'm not sure about that
Test app is available at https://github.com/bompi88/easy-search-leaderboard. The filters are retrieved from ES aggregations. |
c236ca5
to
7e0dffc
Compare
…ions(), cursor.getAggregation(name)
@matteodem thanks for quick merging 😄 have you looked at this PR? some things you like me to add or modify? |
Can you add a useage example of this aggregations functionality to the elasticsearch README? with the component methods and so on. Otherwise good job! |
done ! |
still got a comment open, can you have a look at that? otherwise it's fine to merge for me |
if (collectionScope._indexConfiguration.countUpdateIntervalMs) { | ||
intervalID = Meteor.setInterval(() => { | ||
let newCount; |
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.
@matteodem It seems that the cursor.count()
returns the initial count value set while instantiating the cursor, so I added a optional mongoCount
parameter that is set to false
by the ESCursor
. Is this a satisfiable solution or should I do it otherwise?
any progress on this? |
@matteodem The progress stalled. I'm not sure where I should precede from here? You want me to extend the core's |
@bompi88 I think it'd be much cleaner to put the aggregation logic into the Also the This is all with the goal to keep elastic search related logic out of the core code. Hope it makes sense |
Implementing Elasticsearch aggregations, which are published together with the hits data.
In current implementation, the aggregations are injected in the
body
function like so:and gets accessible on the cursor returned by the
search
method like this: