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

h2: poll ready before using #997

Merged
merged 1 commit into from
May 1, 2024
Merged

Conversation

howardjohn
Copy link
Member

This is "required" per the docs. I don't think it really is, but good to
be safe and follow the docs

This is "required" per the docs. I don't think it really is, but good to
be safe and follow the docs
@howardjohn howardjohn requested a review from a team as a code owner May 1, 2024 22:47
@howardjohn howardjohn added release-notes-none Indicates a PR that does not require release notes. cherrypick/release-1.22 Set this label on a PR to auto-merge it to the release-1.22 branch labels May 1, 2024
@istio-testing istio-testing added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label May 1, 2024
@@ -258,6 +258,9 @@ impl H2ConnectClient {
&mut self,
req: Request<()>,
) -> Result<(SendStream<SendBuf>, h2::RecvStream), Error> {
// "This function must return `Ready` before `send_request` is called"
// We should always be ready though, because we make sure we don't go over the max stream limit out of band.
Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah I saw this in the docs at some point but didn't check if we were doing this in the DIY client.

Better to be correct.

@howardjohn
Copy link
Member Author

/retest

flake is fixed in another pr

@howardjohn
Copy link
Member Author

/retest
#998

@istio-testing istio-testing merged commit faa4f23 into istio:master May 1, 2024
3 of 4 checks passed
@istio-testing
Copy link
Contributor

In response to a cherrypick label: #997 failed to apply on top of branch "release-1.22":

Applying: h2: poll ready before using
Using index info to reconstruct a base tree...
A	src/proxy/h2_client.rs
Falling back to patching base and 3-way merge...
CONFLICT (modify/delete): src/proxy/h2_client.rs deleted in HEAD and modified in h2: poll ready before using. Version h2: poll ready before using of src/proxy/h2_client.rs left in tree.
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 h2: poll ready before using
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

@istio-testing
Copy link
Contributor

In response to a cherrypick label: new issue created for failed cherrypick: #999

howardjohn added a commit to howardjohn/ztunnel that referenced this pull request May 2, 2024
This is "required" per the docs. I don't think it really is, but good to
be safe and follow the docs

(cherry picked from commit faa4f23)
istio-testing pushed a commit that referenced this pull request May 2, 2024
This is "required" per the docs. I don't think it really is, but good to
be safe and follow the docs

(cherry picked from commit faa4f23)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cherrypick/release-1.22 Set this label on a PR to auto-merge it to the release-1.22 branch release-notes-none Indicates a PR that does not require release notes. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants