Add bytes method to Response struct

[laurocaetano]

This patch has a simple change to wrap the reqwest::Response::bytes() into the client Response struct. Additionally I've taken the liberty to do some cosmetic changes in the overall struct - I hope that's fine too.

Thanks @dodomorandi for opening up the issue.

Closes #160

[elasticmachine]

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?

[cla-checker-service]

💚 CLA has been signed

[russcam]

Hi @laurocaetano, please can you sign the CLA so that we can pull this in. Also, would you mind adding an integration test to https://github.com/elastic/elasticsearch-rs/blob/master/elasticsearch/tests/client.rs for a bytes response? The tests can be run with

cargo make  test-elasticsearch

which will spin up an elasticsearch docker instance to run against.

[laurocaetano]

Thanks so much @russcam! I'm going to work on that tomorrow morning :)

It feels good to do open-source again ❤️

[laurocaetano]

I've updated the PR adding the integration tests, thanks for the review!

---

A couple of notes on the integration tests:

The task test-elasticsearch is private, so I only ran the tests with cargo make test.

The integration tests only run when I set TEST_SUITE=platinum: This is due to this condition. I am not sure if that is intended though. The following change would behave as I would expect:

-condition = { env_set = [ "ELASTICSEARCH_URL" ], env = { "TEST_SUITE" = "platinum" } }
+condition = { env_set = [ "ELASTICSEARCH_URL", "TEST_SUITE" ] }

I also get failures when running the cert tests:

    certificate_certificate_validation
    default_certificate_validation
    fail_certificate_certificate_validation
    full_certificate_validation

But I did not have enough time to debug those failures as well. It is probably due to some lack of configuration when running the tests on my machine.

Please let me know what you think about these issues. Probably I am missing something on my configs.

[russcam]

The task test-elasticsearch is private, so I only ran the tests with cargo make test.

My fault, the correct task is cargo make test 👍

The integration tests only run when I set TEST_SUITE=platinum: This is due to this condition. I am not sure if that is intended though.

That is intended because the tests include tests for TLS, which is only set up in the Elasticsearch docker when the TEST_SUITE is platinum. It looks like a change in ac43b52#diff-9375fd04332c86472d7be397ef09428cb86babd8826880a5835bd1d1c1bdbc08R8 changed the default test suite from platinum to free, so it needs to be run with

cargo make test --env TEST_SUITE=platinum

@swallez should the default test suite be changed to platinum, so just cargo make test will run the tests?

I also get failures when running the cert tests:

certificate_certificate_validation
default_certificate_validation
fail_certificate_certificate_validation
full_certificate_validation

Do you see this when running with TEST_SUITE platinum? I suspect the issue is likely the assertions in the tests; the assertions try to accommodate differences across Windows, macOS and Linux. They pass for me locally on Windows and Ubuntu 18.04 through WSL2. I think these can be addressed separately to this PR, if you'd like to open a separate issue with details about your setup?

[laurocaetano]

Do you see this when running with TEST_SUITE platinum? I suspect the issue is likely the assertions in the tests; the assertions try to accommodate differences across Windows, macOS and Linux. They pass for me locally on Windows and Ubuntu 18.04 through WSL2. I think these can be addressed separately to this PR, if you'd like to open a separate issue with details about your setup?

I tried running locally (Ubuntu 20.04) and the cert tests fail. Some of them fail with:

---- full_certificate_ca_chain_validation stdout ----
Error: Error { kind: Http(reqwest::Error { kind: Request, url: Url { scheme: "https", host: Some(Domain("localhost")), port: Some(9200), path: "/", query: None, fragment: None }, source: hyper::Error(Connect, ConnectError("tcp connect error", Os { code: 111, kind: ConnectionRefused, message: "Connection refused" })) }) }

I've created the issue: #167. I will invest a bit more time into it the next days, and understand the real issue in these failures on my machine.

[russcam]

Thanks @laurocaetano.

I re-ran the tests now on Ubuntu 18.04 with WSL 2, and am seeing test failures. The TLS related dependencies of reqwest were updated between 0.10 and 0.11, so I suspect the tests need some adjustment, which we can follow up separately on #167.

[russcam]

Thank you for your contribution @laurocaetano! 👍

[laurocaetano]

Thanks for your support @russcam 👍