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

add ProtocolVersion to ReattachConfig #171

Merged
merged 3 commits into from
Mar 31, 2021

Conversation

mildwonkey
Copy link
Contributor

This PR adds the (already-determined) ProtocolVersion to ReattachConfig. This information was not captured previously, so a caller connecting to a running process had no way of knowing what ProtocolVersion had been negotiated: client.NegotiatedVersion would always return 0.

A client connecting to a plugin process needs to know the resolved ProtocolVersion, so this PR adds that information to the returned reattach config. Prior to this, client.NegotiatedVersion() always returned 0.
@mildwonkey mildwonkey requested review from jbardin and mitchellh March 31, 2021 13:13
Copy link
Member

@jbardin jbardin left a comment

Choose a reason for hiding this comment

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

LGTM

@mildwonkey mildwonkey merged commit 044aadd into master Mar 31, 2021
@mildwonkey mildwonkey deleted the mildwonkey/protov-in-reattach branch March 31, 2021 15:06
kmoe added a commit to hashicorp/terraform-exec that referenced this pull request May 25, 2021
As of go-plugin v1.4.1, the reattach config now contains protocol version information so that protocol version can be negotiated during reattach. Please see hashicorp/go-plugin#171.
kmoe added a commit to hashicorp/terraform-exec that referenced this pull request May 27, 2021
As of go-plugin v1.4.1, the reattach config now contains protocol version information so that protocol version can be negotiated during reattach. Please see hashicorp/go-plugin#171.
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