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

shortcut expander: use discovery helper's cached resource list #1457

Merged
merged 1 commit into from May 7, 2019

Conversation

skriss
Copy link
Member

@skriss skriss commented May 7, 2019

Signed-off-by: Steve Kriss krisss@vmware.com

Fixes #1413
Fixes #1435
Fixes #1414

Change the shortcut expander (which expands things like "po" to pods) to use the discovery helper's cached list of API resources, so it doesn't have to call the discovery client every time there's an expansion to do.

Still doing a little bit of testing, but initial basic tests look good functionally, and make a major performance impact (a sample restore dropped from 45sec to 2sec, a full-cluster backup dropped from 2+ minutes to 11 seconds).

We should revisit the discovery code a little bit post-1.0 as there have been some upstream changes, but happy with this change for now since it's simple and has a big impact.

Signed-off-by: Steve Kriss <krisss@vmware.com>
@skriss skriss requested review from carlisia and nrb May 7, 2019 18:27
@hmcgonig
Copy link

hmcgonig commented May 7, 2019

Thank you again for taking a look at this. I'll try this out locally on my test env as well.

Copy link
Contributor

@carlisia carlisia left a comment

Choose a reason for hiding this comment

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

lgtm 👍

@nrb nrb merged commit 05b8edf into vmware-tanzu:master May 7, 2019
@skriss
Copy link
Member Author

skriss commented May 7, 2019

Oops, wasn't quite done testing here, but we'll see how things look for @hmcgonig and can follow up if any fixes are needed.

@hmcgonig
Copy link

hmcgonig commented May 7, 2019

This looks good on my test cluster, super fast backups. 🎉

Will let you all know if I see any issues once this gets rolled into our main QA cluster tomorrow

@skriss
Copy link
Member Author

skriss commented May 7, 2019

I did some more testing and everything looked ok to me too

@skriss skriss deleted the shortcuts-perf-issue branch May 9, 2019 21:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants