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

TLS Support #233

Merged
merged 1 commit into from
Nov 13, 2023
Merged

TLS Support #233

merged 1 commit into from
Nov 13, 2023

Conversation

paralainer
Copy link
Contributor

Another attempt to add TLS support.
Since last attempt:

  • Merged with master
  • Removed some redundant connection close code
  • Changed approach to integration tests, useTls is now just a parameter for it.
  • Ran pipeline multiple times on Github Actions, it didn't fail even once

@paralainer
Copy link
Contributor Author

@thiagocesarj hopefully this time it won't cause any issues with tests

@thiagocesarj
Copy link
Collaborator

@paralainer thank you for the contribution! I will prepare a release for this feature as version 1.18.0.

@thiagocesarj thiagocesarj merged commit 04cade5 into spotify:master Nov 13, 2023
6 checks passed
@thiagocesarj
Copy link
Collaborator

We had another error.

@thiagocesarj
Copy link
Collaborator

This error doesn't make a lot of sense to me. It is essentially complaining that the handler in the pipeline did not handle the exception. But the change you've made puts SSL in the middle, and it not the last handler in the pipeline.

Also, it seems to happen only when building for master, but not when building for branch. This is a bit mysterious as well.

@paralainer
Copy link
Contributor Author

Oh, I'll have a look into the error, I'm pretty sure it's some problem with test setup, we use that code in prod for a couple of months now in a high load service with no issues

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.

2 participants