-
Notifications
You must be signed in to change notification settings - Fork 733
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
Allow building client by specifying URL #120
Comments
Interesting idea. Is it possible to configure elasticsearch like this or are you using a proxy or something similar? I see two options:
In a way I think best would be to implement both. If url is set, all the others are overwritten, if not, the option set or the default is set. Both should also be possible in the server array for multiple server. What do you think? |
It's probably a common use-case to proxy Elasticsearch HTTP, either for load-balancing or authorization concerns. I think your approach is sensible (url and path). Both would cover this use-case. |
I added some code the to the master repository. Now it should be possible to set path or url. As I don't have a proxy setup here, can you try this out? |
Yep, it works. It requires that the url end with a '/' though. Maybe you could make it friendlier by detecting whether the provided url ends with a '/' and append one when it doesn't. |
Ok, good two know. So there are two options:
It looks like you prefer point 2. The issue I see here, that perhaps we need it one day with /, and than we have some backward compatiblity. One more suggestion: if / is missing, we throw an exception. Instead of "fixing" the code we point out the errors from the beginning. What do you think? |
Well, I very much doubt that someone would mount ES under something else than a 'directory'. It would result in URLs like so: http://eshost/es_search If there's someone out there that does this, then he or she is alone (doing this is simply looking for trouble). I don't see much benefit in supporting a single person's needs. I'd still vote for adding it automatically to help with the 99% use case. My 2 cents... |
I just don't like that I have to fix in my code the code from someone else ;-) Do you think throwing exception is a good idea? |
sure, but doing so will simply push the problem down to your users. Every calling code will have to test the url parameter before passing it in to your library... same thing, different place. |
Sorry to go on with the arguing ;-) I'm not sure with we talk about the same use case. My assumption is: The path is set in a config file or hardcoded into the code. So it is nothing dynamic that changes too often, right? So the developer (not the user) has to add the path. Or how does your usecase look like? |
It's true that the URL may be configured by the actual user of Elastica, but it might not. For example, the Drupal module for using Elasticsearch with SearchAPI has a form where the Drupal end-user types the URL. In this case, the Drupal module would have to validate the URL... no big deal, really... |
I probably go with the option, that the Drupal module has to fix this issue ;-) I hope this is ok for you. |
Can this issue be closed then ? |
Closed |
Some elasticsearch servers may not be rooted on /.
It would be useful in these cases to allow building a client with a URL instead of a host/port combination:
The text was updated successfully, but these errors were encountered: