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

chore: update deps #265

Merged
merged 39 commits into from
Jul 12, 2024
Merged

chore: update deps #265

merged 39 commits into from
Jul 12, 2024

Conversation

nacardin
Copy link
Contributor

@nacardin nacardin commented Jul 2, 2024

  • Update all depencencies to latest versions
  • Timer sleeper only fires once
  • Remove http-client
  • Rename native2_tls feature to native_tls
  • Note: rustls now uses aws-lc instead of ring

Includes #241 and #258

Resolves #239

@morenol
Copy link
Contributor

morenol commented Jul 2, 2024

@nacardin maybe this PR could be helpful #258

@nacardin
Copy link
Contributor Author

nacardin commented Jul 8, 2024

@nacardin maybe this PR could be helpful #258

That seems to change behavior to only fire once. I made a change to preserve existing behavior of firing the timer for every await of the future.

@morenol I just noticed that you tried a similar change in the PR, why did you revert it?

@morenol
Copy link
Contributor

morenol commented Jul 8, 2024

I reverted it because @sehz mentioned me that it should only fire once:

#258 (comment)

Not sure if fluvio or any other crate relies on that multiple fires behavior by the timer

@nacardin nacardin marked this pull request as ready for review July 11, 2024 14:26
@nacardin nacardin requested review from morenol and sehz July 11, 2024 14:26
Copy link
Contributor

@sehz sehz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. few minor nits on changing to anyhow and log level

src/rust_tls.rs Outdated Show resolved Hide resolved
src/rust_tls.rs Outdated Show resolved Hide resolved
src/rust_tls.rs Show resolved Hide resolved
src/rust_tls.rs Outdated Show resolved Hide resolved
src/rust_tls.rs Outdated Show resolved Hide resolved
src/rust_tls.rs Outdated Show resolved Hide resolved
src/rust_tls.rs Outdated Show resolved Hide resolved
src/rust_tls.rs Outdated Show resolved Hide resolved
fluvio-future-derive/Cargo.toml Outdated Show resolved Hide resolved
Copy link
Contributor

@sehz sehz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice. LGTM

@nacardin
Copy link
Contributor Author

@sehz We should also convert native_tls and openssl_tls errors to anyhow?

@nacardin nacardin added this pull request to the merge queue Jul 12, 2024
Merged via the queue into infinyon:master with commit bdf4c3b Jul 12, 2024
8 checks passed
@nacardin nacardin deleted the nacardin/deps branch July 12, 2024 23:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

async_rustls is deprecated we should move to futures-rustls
4 participants