Skip to content

feat: Expose ConnectionString method in Client#57

Merged
kodiakhq[bot] merged 2 commits intomainfrom
connection-string
Jul 3, 2023
Merged

feat: Expose ConnectionString method in Client#57
kodiakhq[bot] merged 2 commits intomainfrom
connection-string

Conversation

@hermanschaaf
Copy link
Member

Exposes a ConnectionString method that can be used to connect to the client via GRPC. It includes the protocol in the string so that GRPC clients can automatically infer the connection type.

@hermanschaaf hermanschaaf requested a review from yevgenypats as a code owner July 3, 2023 12:03
@github-actions github-actions bot added the feat label Jul 3, 2023
case RegistryLocal:
return "unix://" + tgt
case RegistryGithub:
return "unix://" + tgt
Copy link
Contributor

Choose a reason for hiding this comment

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

where you able to test it? that it works on connection creation? I don't remember if it's unix:// or unix:///

Copy link
Member Author

Choose a reason for hiding this comment

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

I tested the unix one yeah, haven't tested grpc:// yet, let me do that now

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah actually grpc:// doesn't work, it says too many colons in address...

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok fixed. In the grpc:// case it should have no prefix, unix:// is treated as a special value by the Go GRPC client.

@yevgenypats yevgenypats added the automerge Add to automerge PRs once requirements are met label Jul 3, 2023
@kodiakhq kodiakhq bot merged commit 45b234e into main Jul 3, 2023
@kodiakhq kodiakhq bot deleted the connection-string branch July 3, 2023 12:43
@cq-bot cq-bot mentioned this pull request Jul 3, 2023
kodiakhq bot pushed a commit to cloudquery/cloudquery that referenced this pull request Jul 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

automerge Add to automerge PRs once requirements are met feat

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants