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

[TEST] Minor REST tests infra cleanup #7723

Closed
wants to merge 1 commit into from

Conversation

@javanna
Copy link
Member

commented Sep 15, 2014

Make the http addresses within the REST client final. It makes no sense to update them before each test if we don't check the version of the nodes again, which would mean adding too much overhead (an additional http call before each test) for no reason. We just reuse the same nodes for the whole suite and check the version once while initializing the client. Would be nice to make the REST client within the execution context final but its initialization still needs to happen after the ElasticsearchIntegrationTest#beforeInternal that assigns GLOBAL_CLUSTER to currentCluster.

[TEST] Minor REST tests infra cleanup
Make the http addresses within the REST client final. It makes no sense to update them before each test if we don't check the version of the nodes again, which would mean adding too much overhead (an additional http call before each test) for no reason. We just reuse the same nodes for the whole suite and check the version once while initializing the client. Would be nice to make the REST client within the execution context final but its initialization still needs to happen after the `ElasticsearchIntegrationTest#beforeInternal` that assigns `GLOBAL_CLUSTER` to `currentCluster`.
@s1monw

This comment has been minimized.

Copy link
Contributor

commented Sep 15, 2014

LGTM

@javanna javanna closed this in ede39ed Sep 15, 2014

javanna added a commit that referenced this pull request Sep 15, 2014
[TEST] Minor REST tests infra cleanup
Make the http addresses within the REST client final. It makes no sense to update them before each test if we don't check the version of the nodes again, which would mean adding too much overhead (an additional http call before each test) for no reason. We just reuse the same nodes for the whole suite and check the version once while initializing the client. Would be nice to make the REST client within the execution context final but its initialization still needs to happen after the `ElasticsearchIntegrationTest#beforeInternal` that assigns `GLOBAL_CLUSTER` to `currentCluster`.

Closes #7723
javanna added a commit that referenced this pull request Sep 15, 2014
[TEST] Minor REST tests infra cleanup
Make the http addresses within the REST client final. It makes no sense to update them before each test if we don't check the version of the nodes again, which would mean adding too much overhead (an additional http call before each test) for no reason. We just reuse the same nodes for the whole suite and check the version once while initializing the client. Would be nice to make the REST client within the execution context final but its initialization still needs to happen after the `ElasticsearchIntegrationTest#beforeInternal` that assigns `GLOBAL_CLUSTER` to `currentCluster`.

Closes #7723
javanna added a commit to javanna/elasticsearch that referenced this pull request Sep 16, 2014
[TEST] Update REST client before each test in our REST tests
In elastic#7723 we removed the `updateAddresses` method from `RestClient` under the assumption that the addresses never change during the suite execution, as REST tests rely on the global cluster. Due to elastic#6734 we restart the global cluster though before each test if there was a failure in the suite. If that happens we do need to make sure that the REST client points to the proper nodes. What was missing before was the http call to verify the es version every time the addresses change, which we do now since we effectively re-initialize the REST client when needed (if the http addresses have changed).
javanna added a commit that referenced this pull request Sep 16, 2014
[TEST] Update REST client before each test in our REST tests
In #7723 we removed the `updateAddresses` method from `RestClient` under the assumption that the addresses never change during the suite execution, as REST tests rely on the global cluster. Due to #6734 we restart the global cluster though before each test if there was a failure in the suite. If that happens we do need to make sure that the REST client points to the proper nodes. What was missing before was the http call to verify the es version every time the addresses change, which we do now since we effectively re-initialize the REST client when needed (if the http addresses have changed).

Closes #7737
javanna added a commit that referenced this pull request Sep 16, 2014
[TEST] Update REST client before each test in our REST tests
In #7723 we removed the `updateAddresses` method from `RestClient` under the assumption that the addresses never change during the suite execution, as REST tests rely on the global cluster. Due to #6734 we restart the global cluster though before each test if there was a failure in the suite. If that happens we do need to make sure that the REST client points to the proper nodes. What was missing before was the http call to verify the es version every time the addresses change, which we do now since we effectively re-initialize the REST client when needed (if the http addresses have changed).

Closes #7737
javanna added a commit that referenced this pull request Sep 16, 2014
[TEST] Update REST client before each test in our REST tests
In #7723 we removed the `updateAddresses` method from `RestClient` under the assumption that the addresses never change during the suite execution, as REST tests rely on the global cluster. Due to #6734 we restart the global cluster though before each test if there was a failure in the suite. If that happens we do need to make sure that the REST client points to the proper nodes. What was missing before was the http call to verify the es version every time the addresses change, which we do now since we effectively re-initialize the REST client when needed (if the http addresses have changed).

Closes #7737

@jpountz jpountz removed the review label Oct 21, 2014

mute pushed a commit to mute/elasticsearch that referenced this pull request Jul 29, 2015
[TEST] Minor REST tests infra cleanup
Make the http addresses within the REST client final. It makes no sense to update them before each test if we don't check the version of the nodes again, which would mean adding too much overhead (an additional http call before each test) for no reason. We just reuse the same nodes for the whole suite and check the version once while initializing the client. Would be nice to make the REST client within the execution context final but its initialization still needs to happen after the `ElasticsearchIntegrationTest#beforeInternal` that assigns `GLOBAL_CLUSTER` to `currentCluster`.

Closes elastic#7723
mute pushed a commit to mute/elasticsearch that referenced this pull request Jul 29, 2015
[TEST] Update REST client before each test in our REST tests
In elastic#7723 we removed the `updateAddresses` method from `RestClient` under the assumption that the addresses never change during the suite execution, as REST tests rely on the global cluster. Due to elastic#6734 we restart the global cluster though before each test if there was a failure in the suite. If that happens we do need to make sure that the REST client points to the proper nodes. What was missing before was the http call to verify the es version every time the addresses change, which we do now since we effectively re-initialize the REST client when needed (if the http addresses have changed).

Closes elastic#7737
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.