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

Remove castore dependency #274

Merged
merged 2 commits into from
Aug 6, 2024
Merged

Conversation

wojtekmach
Copy link
Contributor

@wojtekmach wojtekmach commented Jun 10, 2024

Closes #178.

Mint was recently updated to automatically use OS trust store: elixir-mint/mint#435.

If we are on OTP 24, we'd receive this error:

image

Same error when using OTP 25+ but the OS happens not to have certs:

image

I believe the error is good enough however I'm happy to improve it of course!

Copy link
Owner

@sneako sneako left a comment

Choose a reason for hiding this comment

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

Thanks @wojtekmach !

@wojtekmach
Copy link
Contributor Author

wojtekmach commented Jun 10, 2024

@sneako actually let's hold off on this for a second, perhaps we should set Mint version requirement to >= 1.6.1 so we always pick a Mint which would use OS certs?

@PragTob
Copy link

PragTob commented Jul 24, 2024

Less dependencies is always great 👀

Any way to help this move along? Looks like one failing test 👀

@wojtekmach
Copy link
Contributor Author

The failing line is this one:

assert_recv_frames([rst_stream(stream_id: ^stream_id, error_code: :no_error)])

I have determined it started failing after this Mint change, elixir-mint/mint#434, which makes sense, Mint stopped sending frames under some circumstances and the test is expecting these. Perhaps it's as simple as updating the test?

cc @v0idpwn

@@ -33,8 +33,7 @@ defmodule Finch.MixProject do

defp deps do
[
{:mint, "~> 1.6"},
{:castore, "~> 0.1 or ~> 1.0"},
{:mint, "~> 1.6.2 or ~> 1.7"},
Copy link
Contributor Author

@wojtekmach wojtekmach Aug 5, 2024

Choose a reason for hiding this comment

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

Mint is using OS trust store since 1.6.1. I thought I'd require the latest available version at the moment, 1.6.2: https://github.com/elixir-mint/mint/blob/main/CHANGELOG.md#v162.

@sneako sneako merged commit 80c2d42 into sneako:main Aug 6, 2024
2 checks passed
@sneako
Copy link
Owner

sneako commented Aug 6, 2024

❤️

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.

Use built-in cacerts
3 participants