Skip to content

Add h1-client-rustls feature #88

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 5 commits into from
Mar 8, 2021

Conversation

JEnoch
Copy link
Contributor

@JEnoch JEnoch commented Mar 8, 2021

Description

  • Add a h1-client-rustlsfeature, allowing to use rustls instead of OpenSSL for HTTPS (and thus being more portable to various platforms)
  • Bump surf to 2.2.0 (to use surf/h1-client-rustls feature)
  • Add Choice of HTTP backend section in README.md, describing the different features
  • Update rust.yml workflow to test a matrix of HTTP backends, instead of using --all-features that activate all, but ends with only curl-client to be tested (as per surf implementation)

Checklist

  • Formatted code using cargo fmt --all
  • Linted code using clippy cargo clippy --all-targets --all-features -- -D warnings
  • Updated README.md using cargo readme -r influxdb -t ../README.tpl > README.md
  • Reviewed the diff. Did you leave any print statements or unnecessary comments?
  • Any unfinished work that warrants a separate issue captured in an issue with a TODO code comment

@JEnoch
Copy link
Contributor Author

JEnoch commented Mar 8, 2021

I removed the testing of wasm-client feature, since it's not working out-of-the box.
It probably requires the wasm-pack environment, similarly to what's done in surf's ci.yaml.

@JEnoch
Copy link
Contributor Author

JEnoch commented Mar 8, 2021

I'm not sure yet why the tests are failing using h1-client.
The same tests are successful on my localhost (also running against InfluxDB 1.8 Docker containers) !
I'm trying to debug this in a side branch in my fork.

@JEnoch
Copy link
Contributor Author

JEnoch commented Mar 8, 2021

Test failures using h1_client where caused by async_h1 resolving localhost as [::1]:8086 (IPv6) instead of 127.0.0.1(IPv4) as hyper does.
Not sure why the connection to localhost using IPv6 fails on Github platform, while it succeeds on my host...
But I don't think that's a blocker for this PR. I changed the tests to use 127.0.0.1 instead of localhost.

@JEnoch
Copy link
Contributor Author

JEnoch commented Mar 8, 2021

@Empty2k12 : I think this PR is ready for review and possible merge.

@Empty2k12
Copy link
Collaborator

Empty2k12 commented Mar 8, 2021

@JEnoch Thanks for this PR, looking great so far! Does indeed look like a typo. Will merge when that's resolved.

@Empty2k12 Empty2k12 merged commit 9a08144 into influxdb-rs:master Mar 8, 2021
@Empty2k12
Copy link
Collaborator

Thanks a lot @JEnoch

@Empty2k12
Copy link
Collaborator

Hello! I just published a change to crates.io containing your change as version 0.4.0. Thanks again for your contribution!

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants