#11 - specify elasticsearch transport parameter and custom auth usera… #17
Conversation
|
@rfarley3 would you help here to move forward. |
rfarley3
left a comment
rfarley3
left a comment
There was a problem hiding this comment.
This is a great idea! However, I feel that there is a bit of mixture of the needs for a requests.session and needs for the custom useragent object. It is also not applied to all http requests (there is a post that isn't modified later in the code).
If we move the pkg manip/loading to the init and reduce duck typing, then I feel that things would clear up.
| --index INDEX Kibana index to work on | ||
| --transport-class TRANSPORT_CLASS | ||
| Elastic search transport class parameter | ||
| --map-ua MAP_UA Over-ride default http user-agent with a custom, |
There was a problem hiding this comment.
Why map-ua? How about user-agent?
| self.index, | ||
| self._index_pattern, | ||
| self._host, | ||
| self.map_ua, |
There was a problem hiding this comment.
Why do the mappings need UAs, but the non-mappings calls don't?
There was a problem hiding this comment.
Would it help if I made both the Mappings and Manager class use the Elasticsearch API instead of Mappings using requests and Manager using ES?
| transport_class_obj = getattr(mod, my_class) | ||
| self.es = Elasticsearch( | ||
| [{'host': self._host_ip, 'port': self._host_port}], transport_class=transport_class_obj) | ||
| else: |
There was a problem hiding this comment.
If transport_class is false, then self.es is never set. Is this indent wrong?
| self.index = index | ||
| self._index_pattern = index_pattern | ||
| self._host = host | ||
| self.map_ua = map_ua, |
There was a problem hiding this comment.
Let's call the setting/conversion function here once, to avoid relying on isinstance of str and leverage a single session instead of one session per request. Also, then instead of self.get_map_ua() we can call the attribute directly.
| def pr_err(self, msg): | ||
| print('[ERR] Mapping %s' % msg) | ||
|
|
||
| def get_map_ua(self): |
There was a problem hiding this comment.
There are too many types for this single variable, it's a string, a custom ua obj, or a requests.session. I feel like the user agent concept is being blended with the sessions usage. Could we separate out or clarify those needs?
2 participants
HI I have added my changes for review to specify a custom transport and ua class. We have a usecase where in instead of ES auth we are using something custom and ES is sitting behind a authentication proxy so queries should have some token added to them.
Users who are having similar requirements now can write their custom plugins and override.