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

[vtctldclient] vtctldclient generator #7238

Merged
merged 11 commits into from
Jan 5, 2021

Conversation

ajm188
Copy link
Contributor

@ajm188 ajm188 commented Jan 3, 2021

Description

This adds a code generator to produce the boilerplate "if underlying connection is nil, return closed; otherwise, call underlying method" for every unary RPC in the vtctld client. It does not handle streaming RPCs (yet) because we do not need streaming RPCs (yet)

Related Issue(s)

Checklist

  • Should this PR be backported? No.
  • Tests were added or are not required N/A
  • Documentation was added or is not required N/A

Deployment Notes

Impacted Areas in Vitess

Components that this PR will affect:

  • Query Serving
  • VReplication
  • Cluster Management
  • Build

Signed-off-by: Andrew Mason <amason@slack-corp.com>
Signed-off-by: Andrew Mason <amason@slack-corp.com>
Signed-off-by: Andrew Mason <amason@slack-corp.com>
Signed-off-by: Andrew Mason <amason@slack-corp.com>
Signed-off-by: Andrew Mason <amason@slack-corp.com>
Signed-off-by: Andrew Mason <amason@slack-corp.com>
Signed-off-by: Andrew Mason <amason@slack-corp.com>
Signed-off-by: Andrew Mason <amason@slack-corp.com>
@ajm188 ajm188 requested a review from doeg as a code owner January 3, 2021 13:13
Copy link
Contributor

@doeg doeg left a comment

Choose a reason for hiding this comment

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

This is so cool!!! Magical, even. (In a good way!) Thank you for doing this!

I pulled your branch and re-generated client_gen.go and it looks good/worked great. (The automatic VS Code integration is so nice, too!)

My only suggestion would be that maybe we could/should add a Makefile target, but I leave that up to you. :)

Signed-off-by: Andrew Mason <amason@slack-corp.com>
@ajm188
Copy link
Contributor Author

ajm188 commented Jan 3, 2021

I was just thinking about how I should have added a Makefile target. Good call.

Signed-off-by: Andrew Mason <amason@slack-corp.com>
@ajm188 ajm188 requested a review from sougou as a code owner January 3, 2021 16:14
@@ -0,0 +1,19 @@
# Copyright 2020 The Vitess Authors.
Copy link
Contributor

Choose a reason for hiding this comment

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

Oh... right, it's 2021 now. 😭 Haha. I guess this applies to all the files in this branch.

Signed-off-by: Andrew Mason <amason@slack-corp.com>
@ajm188 ajm188 requested a review from rohit-nayak-ps January 5, 2021 23:37
Copy link
Member

@deepthi deepthi left a comment

Choose a reason for hiding this comment

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

Very cool indeed! 💯

@deepthi deepthi merged commit f51ac19 into vitessio:master Jan 5, 2021
@askdba askdba added this to the v9.0 milestone Jan 6, 2021
ajm188 pushed a commit to tinyspeck/vitess that referenced this pull request Jan 10, 2021
…ator

[vtctldclient] vtctldclient generator
@ajm188 ajm188 deleted the am_vtctldclient_generator branch January 14, 2021 15:40
setassociative pushed a commit to tinyspeck/vitess that referenced this pull request Mar 11, 2021
…ator

[vtctldclient] vtctldclient generator
@ajm188 ajm188 added Component: Build/CI Type: Enhancement Logical improvement (somewhere between a bug and feature) labels May 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Build/CI Type: Enhancement Logical improvement (somewhere between a bug and feature)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants