Skip to content
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

update/remove_depricated_type_from_mget #2397

Merged

Conversation

pschoffer
Copy link
Contributor

@pschoffer pschoffer commented Oct 8, 2021

Description of the Change

Avoids the deprecation warnings:

Warning: 299 Elasticsearch-7.8.0-757314695644ea9a1dc2fecd26d1a43856725e65 "[types removal] Specifying types in multi get requests is deprecated." 

In 7.X using mapping types is deprecated. Types are not used anymore (in previous versions of ES types would be used for posts, users and so on) we are using different indexes instead.

In get_documnets we seem to refect it as there is different URL used for > 7.0 ES version. This variant is not using the custom type, but instead, it tries to use _doc as kinda default type. This works, but the correct URL in this version for _mget is to omit the document type altogether. This change does just that.

Alternate Designs

Benefits

Possible Drawbacks

Verification Process

  1. Add a debug print in Elasticsearch.php:1136:
			if ( true === apply_filters( 'ep_intercept_remote_request', false ) ) {
				/**
				 * Filter intercepted request
				 *
				 * @hook ep_do_intercept_request
				 * @param {array} $request New remote request response
				 * @param  {array} $query Remote request arguments
				 * @param  {args} $args Request arguments
				 * @param  {int} $failures Number of failures
				 * @return {array} New request
				 */
				$request = apply_filters( 'ep_do_intercept_request', new WP_Error( 400, 'No Request defined' ), $query, $args, $failures );
			} else {
				$request = wp_remote_request( $query['url'], $args ); // try the existing host to avoid unnecessary calls.
			}

			var_dump($request['headers']); // <---- THIS
  1. Run multi get
wp eval "\ElasticPress\Indexables::factory()->get('post')->multi_get([1,2]);"

object(Requests_Utility_CaseInsensitiveDictionary)#3210 (1) {
  ["data":protected]=>
  array(4) {
    ["warning"]=>
    string(136) "299 Elasticsearch-7.8.0-757314695644ea9a1dc2fecd26d1a43856725e65 "[types removal] Specifying types in multi get requests is deprecated.""
    ["content-type"]=>
    string(31) "application/json; charset=UTF-8"
    ["content-encoding"]=>
    string(4) "gzip"
    ["content-length"]=>
    string(4) "1334"
  }
}


Checklist:

  • I have read the CONTRIBUTING document.
  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my change.
  • All new and existing tests passed.

Applicable Issues

Changelog Entry

Fixed: Replaced deprecated URL for multiple documents get from ElasticSearch.

@felipeelia felipeelia added this to the 3.6.4 milestone Oct 8, 2021
@felipeelia felipeelia merged commit 85a9e1a into 10up:develop Oct 18, 2021
@rebeccahum rebeccahum deleted the update/remove_depricated_type_from_mget branch November 2, 2021 16:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants