elixir-grpc / grpc

An Elixir implementation of gRPC
https://hex.pm/packages/grpc
Apache License 2.0
1.36k stars 210 forks source link

Update gun to 2.0 #301

Closed carrascoacd closed 1 year ago

carrascoacd commented 1 year ago

Context

Gun 2.0 has been released. As a consequence, the dependency resolution when using https://github.com/elixir-grpc/grpc is broken.

In order to remove the temporal fork of gun and use the official gun library, we want to make sure that:

  1. gun depends on cowlib 2.12.0
  2. cowboy depends on cowlib 2.12.0
  3. grpc depends indirectly on on cowlib 2.12.0

I'm also taking the chance for releasing the minor version of the dependency so we are not so strict.

Related to https://github.com/ninenines/cowboy/pull/1601 @essen @polvalente

nulian commented 1 year ago

cowboy with cowlib is finally updated so this can be merged.

polvalente commented 1 year ago

@carrascoacd (cc @nulian) The tests are currently failing on CI due to some SSL validation issues. Any ideas on how to fix that? Locally everything works.

carrascoacd commented 1 year ago

I'm afraid it is because certs_keys were added in OTP 25 so probably the current tests are not working. I've added a commit to try to solve it but now the previous from 25 failed. I think we will need to create different tests to distinguish it and change the CI configuration to run them depending on the version.

polvalente commented 1 year ago

I'm afraid it is because certs_keys were added in OTP 25 so probably the current tests are not working. I've added a commit to try to solve it but now the previous from 25 failed. I think we will need to create different tests to distinguish it and change the CI configuration to run them depending on the version.

What's weird to me is that Elixir 1.14 + OTP 25 works fine locally for me, without changing anything.

carrascoacd commented 1 year ago

Yes, it is quite weird. Maybe I'm wrong and it is backwards compatible. Can you check the runtime GH uses in the tests to use exactly that instead of asdf based?

polvalente commented 1 year ago

Great news, it seems that the SSL issues appear on Erlang 25.3, so using 25.1.x (which is the same I had locally) solves this for now, so we'll be able to merge

carrascoacd commented 1 year ago

Great! Good job