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 honor_cipher_order option #517

Closed
wants to merge 1 commit into from

Conversation

tombruijn
Copy link
Member

A new hackney version has been released which no longer sets
honor_cipher_order to true by default. This means we can remove our
workaround. Make sure to upgrade hackney for AppSignal to work.

Workaround introduced in #516
Related issue #512

@tombruijn tombruijn self-assigned this Sep 23, 2019
@tombruijn
Copy link
Member Author

Waiting for a new hackney release to merge and publish this in a new patch release.

A new hackney version has been released which no longer sets
`honor_cipher_order` to `true` by default. This means we can remove our
workaround. Make sure to upgrade hackney for AppSignal to work.
@tombruijn tombruijn force-pushed the remove-honor_cipher_order branch from b204685 to 2a7b097 Compare September 23, 2019 19:56
@tombruijn tombruijn marked this pull request as ready for review September 24, 2019 06:18
@tombruijn
Copy link
Member Author

Hackney released a new version with the fix: https://hex.pm/packages/hackney

This PR can now be reviewed.

@jeffkreeftmeijer
Copy link
Member

Discussed with Tom; we're closing this to allow users to keep using an older version of Hackney, even on OTP 22.1, by keeping our workaround in. The workaround only applies to our HTTP calls, and doesn't affect any other use of Hackney, and reverts the honor_cipher_order configuration to Erlang's (and now Hackney 1.5.2's) default:

honor_cipher_order() = boolean()
If set to true, use the server preference for cipher selection. If set to false (the default), use the client preference.

To allow us to remove this workaround, we might require a stricter Hackney dependency in the future.

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

Successfully merging this pull request may close these issues.

2 participants