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

fix: use recently-added ConnectRPC20 compat variant #85

Closed
wants to merge 2 commits into from

Conversation

johnstcn
Copy link
Member

@johnstcn johnstcn commented Jul 4, 2024

Use the compat method added in coder/coder#13778
This should allow using the newer endpoint with Coder version 2.9.

@johnstcn johnstcn self-assigned this Jul 4, 2024
@johnstcn johnstcn requested review from sreya and spikecurtis July 4, 2024 14:52

require (
cdr.dev/slog v1.6.2-0.20240126064726-20367d4aede6
github.com/coder/coder/v2 v2.12.0
github.com/coder/coder/v2 v2.10.1-0.20240703121105-f6639b788f7b
Copy link
Member Author

Choose a reason for hiding this comment

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

@@ -4,11 +4,11 @@ go 1.22.4

// There are a few minor changes we make to Tailscale that we're slowly upstreaming. Compare here:
// https://github.com/tailscale/tailscale/compare/main...coder:tailscale:main
replace tailscale.com => github.com/coder/tailscale v1.1.1-0.20240530071520-1ac63d3a4ee3
replace tailscale.com => github.com/coder/tailscale v1.1.1-0.20240702054557-aa558fbe5374
Copy link
Member Author

Choose a reason for hiding this comment

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

copied from coder/coder go.mod

Copy link

@spikecurtis spikecurtis left a comment

Choose a reason for hiding this comment

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

c.f. inline comment; however, I don't need to review again after it is dealt with.

if err != nil {
logger.Error(ctx, "connect err", slog.Error(err))
continue
}
conn = c.DRPCConn()

Choose a reason for hiding this comment

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

client.ConnectRPC20 returns the equivalent to the arpc below. Grabbing the underlying Conn and then recreating the RPC client isn't necessary and breaks the type-checking that should prevent you from using RPCs that aren't present in v2.0 of the Agent API

@sreya sreya closed this Nov 27, 2024
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.

3 participants