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

Fixes #1042 Fix PHP preloading #1051

Merged
merged 1 commit into from
Jul 16, 2020
Merged

Conversation

annuh
Copy link
Contributor

@annuh annuh commented Jul 8, 2020

This PR should fix the following warning when using PHP autoloading with Symfony:

Warning: Cannot declare class \Elasticsearch\Endpoints\Cluster\Nodes\HotThreads, because the name is already in use in /var/www/html/vendor/elasticsearch/elasticsearch/src/autoload.php on line 11

Warning: Cannot declare class \Elasticsearch\Endpoints\Cluster\Nodes\Info, because the name is already in use in /var/www/html/vendor/elasticsearch/elasticsearch/src/autoload.php on line 12

Warning: Cannot declare class \Elasticsearch\Endpoints\Cluster\Nodes\ReloadSecureSettings, because the name is already in use in /var/www/html/vendor/elasticsearch/elasticsearch/src/autoload.php on line 13

Warning: Cannot declare class \Elasticsearch\Endpoints\Cluster\Nodes\Stats, because the name is already in use in /var/www/html/vendor/elasticsearch/elasticsearch/src/autoload.php on line 14

Warning: Cannot declare class \Elasticsearch\Endpoints\Cluster\Nodes\Usage, because the name is already in use in /var/www/html/vendor/elasticsearch/elasticsearch/src/autoload.php on line 15

Warning: Cannot declare class \Elasticsearch\Endpoints\Cluster\Settings\Get, because the name is already in use in /var/www/html/vendor/elasticsearch/elasticsearch/src/autoload.php on line 16

@elasticmachine
Copy link
Collaborator

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

@ezimuel
Copy link
Contributor

ezimuel commented Jul 16, 2020

@annuh thanks for this PR. I have only one question, why is false the autoloading in class_exists()?

@ezimuel ezimuel added this to the 7.9.0 milestone Jul 16, 2020
@annuh
Copy link
Contributor Author

annuh commented Jul 16, 2020

@annuh thanks for this PR. I have only one question, why is false the autoloading in class_exists()?

I though $autoload = false would be a bit faster, but it should work in both ways. See also: https://www.php.net/manual/en/function.class-exists.php#113852

Do you prefer this check without the $autoload = true option?

@ezimuel
Copy link
Contributor

ezimuel commented Jul 16, 2020

If #1042 issue is related to the preloading feature of PHP I think we can use false since it means the alias has alrean been loaded during preload. I would like to test it before merge. I asked for more information about the preload configuration here #1042 (comment).
@annuh can you test if it works with true and false as second parameter of class_exists()? Thanks.
I'll also try to build a test case for that.

@ezimuel
Copy link
Contributor

ezimuel commented Jul 16, 2020

@annuh I did some test using the opcache.preload feature with true and false. It seems ok in both cases. We can use false, as you suggested. Thanks for the PR. I'll merge soon.

@ezimuel ezimuel merged commit ff764e2 into elastic:7.8 Jul 16, 2020
ezimuel pushed a commit that referenced this pull request Jul 16, 2020
@ezimuel
Copy link
Contributor

ezimuel commented Aug 18, 2020

Just released elasticsearch-php 7.9.0 with #1051 included

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