Add the API compatibility header in v8 #1233
Conversation
|
@technige the PHP test are failing because we a license issue running Elasticsearch using |
| @@ -106,10 +107,12 @@ protected function addQueryString(string $url, array $params, array $keys): stri | |||
| */ | |||
| protected function bodySerialize($body, string $contentType): string | |||
| { | |||
| if (strpos($contentType, 'application/x-ndjson') !== false) { | |||
| if (strpos($contentType, 'application/x-ndjson') !== false || | |||
There was a problem hiding this comment.
Is there ever a positive case when strpos(...) wouldn't just be 0, rather than just non-false? It's a weird (almost non-existent) edge case, but snapplication/x-ndjson or something similar would get through this?
So is there any reason why we couldn't change !== false to === 0, to tighten it up?
Alternatively, use str_starts_with to be even clearer.
There was a problem hiding this comment.
Because 0 means application/x-ndjson is present in $contentType as the beginning of the string. Using false we are more safe, becuase we are sure that no substring is present.
|
@ezimuel Shouldn't this be optional, having a flag to control it? We are currently trying to use client v.8 with server v.7. |
This PR adds the API compatibility header in client v8. This should solve issue #1221