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.

Sign up for GitHub

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

Jump to bottom

http_helper.HTTPDoWithRetry does not support HTTP/2 with TLS config #1139

Open
cbascom opened this issue Jun 7, 2022 · 3 comments · May be fixed by #1140
Open

http_helper.HTTPDoWithRetry does not support HTTP/2 with TLS config #1139

cbascom opened this issue Jun 7, 2022 · 3 comments · May be fixed by #1140
Labels
awaiting response Waiting for a response or more data from issue reporter question

Comments

@cbascom
Copy link

cbascom commented Jun 7, 2022

Currently the http_helper.HTTPGetWithRetry method supports the use of HTTP/2 even when TLS config is provided as a result of changes made in PR#560. Unfortunately, the same changes are needed for the http_helper.HTTPDoWithRetry method. PR#987 was opened to address this issue but seems to be stuck waiting to be rebased and reviewed at this point.

Can someone advise on next steps to get PR#987 looked at? I'm not sure if @ngyl88 is still available to rebase that PR and address any review comments, but if not I would be happy to open a new PR as well if needed.
@denis256
Copy link
Member

denis256 commented Jun 9, 2022

Hi,
I think the most simples approach is to open another PR that will include all required fixes

@denis256 denis256 added question awaiting response Waiting for a response or more data from issue reporter labels Jun 9, 2022
@cbascom cbascom linked a pull request Jun 11, 2022 that will close this issue
Open
@cbascom
Copy link
Author

cbascom commented Jul 6, 2022

Thanks @denis256, I have created the new PR, not sure if you can assist in getting it reviewed?

@cbascom
Copy link
Author

cbascom commented Sep 25, 2022

@denis256 Can you please review #1140 so this can get merged? Would like to stop having to hack around this issue if possible.

@ngyl88 ngyl88 mentioned this issue Oct 18, 2022
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
awaiting response Waiting for a response or more data from issue reporter question
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Use http.DefaultTransport with custom TLS config cbascom/terratest
2 participants
@cbascom @denis256