Add support for Gun 2.0 - Feat: update hackney #625
Conversation
carrascoacd
changed the title
test/tesla/adapter/gun_test.exs
Outdated
| @@ -186,14 +186,14 @@ defmodule Tesla.Adapter.GunTest do | |||
| assert response.status == 500 | |||
| end | |||
|
|
|||
| test "error on socks proxy" do | |||
| test "error waiting for response on socks proxy" do | |||
There was a problem hiding this comment.
@yordis maybe we just can get rid of this test since socks proxy is already supported.
There was a problem hiding this comment.
I do not see why not, but what about v1 people? I am just wondering if this will impose a major Tesla version bump.
There was a problem hiding this comment.
Oh you are right about the v1. In my opinion, I'd go for enforcing >= 2.0 and release a major version of tesla.
There was a problem hiding this comment.
It is possible, and we did that for poison 3.x but I just noticed that this test was removed in #584 🧐
There is also missing test checking current codebase against 1.0.0 tests 🤔
There was a problem hiding this comment.
Oh! I see si you rely on a test mix.lock file, dark magic 🧙 . Cool, I'll do it in that way 😀
There was a problem hiding this comment.
Ok guys. Please take a look again. I thought about defining two modules based on the version and duplicating the whole files but, from now, I think that we can keep it like that as the differences are very small.
Notice that I'm doing the check more explicitly, using Version instead of function_exported?.
There was a problem hiding this comment.
PS: please, could you re-run the failed tests? I think they are flaky ones [test/tesla/middleware/retry_test.exs]. Edit: it looks like shared the RetryTest is using an adapter that is not able to be shared in concurrent runs, I've selected the only test that we should care about in gun 1.0in the workflow.
I've also tried to add tests for Elixir 1.15 but it looks like there is something still pending to be done, according to the errors: https://github.com/elixir-tesla/tesla/actions/runs/6517890595/job/17702775428?pr=625
|
What is the minimal required version of |
carrascoacd
changed the title
carrascoacd
changed the title
Co-authored-by: Yordis Prieto <[email protected]>
|
@carrascoacd I just made a mistake ... do you mind opening the PR against We can close the |
* chore(deps): bump gun from 1.3.3 to 2.0.1 Bumps [gun](https://github.com/ninenines/gun) from 1.3.3 to 2.0.1. - [Commits](ninenines/gun@1.3.3...2.0.1) --- updated-dependencies: - dependency-name: gun dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <[email protected]> Signed-off-by: Yordis Prieto <[email protected]> * fix on exit assertion * Add support for Gun 2.0 - Feat: update hackney (#625) * Remove notice for socks proxy support and use gun:headers instead of gun:request * Fix lint errors * Add backwards-compatibility for gun1 * Use gun 1.x * Fix linting * Add Elixir 1.15 and OTP 26.1 to test matrix * Fix matrix * Fix matrix typo * Update hackney * Add concurrency to avoid shared access * Run only gun tests to avoid concurrency problems * Make explicit the gun job * Leave tests for Elixir 1.15 and OTP 26 for another iteration * Fix warnings and improve doc * Update .github/workflows/test.yml Co-authored-by: Yordis Prieto <[email protected]> * Move start_link to a separate process in order to avoid race conditions --------- Co-authored-by: Yordis Prieto <[email protected]> --------- Signed-off-by: dependabot[bot] <[email protected]> Signed-off-by: Yordis Prieto <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Antonio <[email protected]>
|
Thanks! Nothing to sorry about. |
Solves the compatibility problems detected on Gun 2.0 #587 cc/ @yordis