Removed the product check for client API < 8 #231
Conversation
lib/Search/Elasticsearch.pm
Outdated
| @@ -64,6 +65,9 @@ sub new { | |||
| $plugin_class->_init_plugin($params); | |||
| } | |||
|
|
|||
| # Get major version of client API | |||
| $Search::Elasticsearch::API_VERSION = substr($params->{client}->{api_version}, 0, index($params->{client}->{api_version}, '_')); | |||
There was a problem hiding this comment.
The use of a global variable for this seems suboptimal, as it's an attribute specific to the client being used for this object. This would get overwritten every time a different version of the client is constructed.
There was a problem hiding this comment.
Good catch, even if this apply only if you have multiple instances of the client. I need to find a way to access the API version of the client from process_response() in Search::Elasticsearch::Role::Cxn. I'm looking into it.
|
@Grinnz Apart from the improvement that you suggested, did you try the fix? Thanks. |
|
@stuartskelton thanks for the feedback. I will try merge this asap and have a release in place. |
|
@Grinnz I finally proposed a solution that avoids using global variable. I created a |
|
Looks reasonable to me. I will try it out soon |
|
same here it looks reasonable to me. |
1 similar comment
|
same here it looks reasonable to me. |
|
I've tested Search-Elasticsearch-8.00 with this patch with elasticsearch server 6.8.23 and everything looks good. |
This PR should solve #227 removing the
x-elastic-productcheck for API version < 8.