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

Execute SSLHelper.validate as blocking #4441

Closed
wants to merge 2 commits into from

Conversation

MikeEdgar
Copy link

Motivation:

Proposed to fix #4439

@MikeEdgar MikeEdgar marked this pull request as ready for review July 21, 2022 13:27
Copy link
Member

@vietj vietj left a comment

Choose a reason for hiding this comment

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

a couple of changes to do

@MikeEdgar
Copy link
Author

I think the use of CountDownLatch in TCPServerBase is also going to be a problem. I'm still looking into that.

@MikeEdgar MikeEdgar requested a review from vietj July 26, 2022 13:46
@MikeEdgar
Copy link
Author

@vietj, I'm curious to get your thinking on these changes. I realize there needs to be a certain amount of caution with changes to these classes/methods.

@vietj
Copy link
Member

vietj commented Aug 9, 2022

it looks good overall but there are a few things that need to be addressed

@MikeEdgar
Copy link
Author

it looks good overall but there are a few things that need to be addressed

Thanks, let me know what's needed when you have an opportunity. No rush from my perspective.

@vietj
Copy link
Member

vietj commented Aug 23, 2022

I will actually reuse this in an general improvement of the SSLHelper because there are other SSLHelper related work that should be done.

@vietj
Copy link
Member

vietj commented Aug 30, 2022

@MikeEdgar please have a look at #4468

@MikeEdgar
Copy link
Author

@vietj , now that #4468 is the path forward on this, should we just close this one?

@vietj
Copy link
Member

vietj commented Sep 9, 2022

yes, I'll credit you in the orignal issue.

@vietj vietj added the duplicate label Sep 9, 2022
@vietj vietj closed this Sep 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Noisy logging from TCPServerBase#listen during startup
2 participants