Skip to content

Conversation

hkethi002
Copy link
Contributor

@hkethi002 hkethi002 commented Aug 7, 2017

Fixes #887
Returns list of nodes of all containers that are returned from a given search query
/dataexplorer/search/nodes

Review Checklist

  • Tests were added to cover all code changes
  • Documentation was added / updated
  • Code and tests follow standards in CONTRIBUTING.md

@hkethi002 hkethi002 requested a review from nagem August 7, 2017 19:39
size = self.search_size(return_type)
body = {
"size": 0,
"_source": ["session._id"],
Copy link
Contributor

Choose a reason for hiding this comment

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

we'll want this to be based on return_type, I'm sure this is just a hard code you missed taking out.

return self.get_file_nodes(return_type, filters, search_string)

size = self.search_size(return_type)
body = {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we could just call "_construct_query" and change the "_source" key?

}
}
}

Copy link
Contributor

Choose a reason for hiding this comment

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

and this would call "_construct_file_query" ?

nodes = []
results = helpers.scan(client=config.es, query={'query': query}, scroll='5m', size=1000, index='data_explorer', doc_type='flywheel', _source=[return_type+'._id'])
log.debug(results)
for result in results:
Copy link
Contributor

Choose a reason for hiding this comment

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

and we'd want to set the source whitelist

@nagem
Copy link
Contributor

nagem commented Aug 28, 2017

Changes look good! I was originally confused about removing part of the by_container aggregation, but ordering the results is unnecessary for this type of endpoint.

@hkethi002 hkethi002 merged commit 5c96fc5 into master Aug 28, 2017
@hkethi002 hkethi002 deleted the node-search branch August 28, 2017 17:13
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