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

Local examples should use pre-built vtadmin-web #11539

Closed
deepthi opened this issue Oct 19, 2022 · 3 comments · Fixed by #13262
Closed

Local examples should use pre-built vtadmin-web #11539

deepthi opened this issue Oct 19, 2022 · 3 comments · Fixed by #13262

Comments

@deepthi
Copy link
Member

deepthi commented Oct 19, 2022

Feature Description

Right now the local example builds vtadmin-web. This is fine for developers (though it slows startup). However, we package the examples and ship them with the binaries.
We should build vtadmin-web in the release script and package it along with the other binaries. Then the Getting Started guides on the website can be changed to use the package.
Local builds via make already build vtadmin-web, so there's no need to rebuild it when running the example locally either.

Use Case(s)

  • Users who download the binaries should not have to run through a build to be able to use vtadmin.
  • Reduce time taken to run local example in development env.
@deepthi deepthi added Type: Feature Needs Triage This issue needs to be correctly labelled and triaged labels Oct 19, 2022
@deepthi deepthi added this to the v15.0 milestone Oct 19, 2022
@deepthi deepthi added Component: Examples and removed Needs Triage This issue needs to be correctly labelled and triaged labels Oct 19, 2022
@deepthi
Copy link
Member Author

deepthi commented Oct 19, 2022

Local builds via make already build vtadmin-web, so there's no need to rebuild it when running the example locally either.

This is not the case right now. make all does not call vtadmin_web_install, it should.

That target also needs refining. It currently reads

vtadmin_web_install:
	cd web/vtadmin && npm install

which means it will leave you in web/vtadmin after running the build.
101_initial_cluster.sh does it better

npm --prefix $web_dir --silent install

@deepthi deepthi mentioned this issue Oct 19, 2022
100 tasks
@deepthi
Copy link
Member Author

deepthi commented Oct 19, 2022

Two examples are affected:

% find . -name vtadmin-up.sh
./local/scripts/vtadmin-up.sh
./region_sharding/scripts/vtadmin-up.sh

@deepthi deepthi modified the milestones: v15.0, v16.0 Oct 20, 2022
@deepthi
Copy link
Member Author

deepthi commented Oct 20, 2022

It's not yet clear what the best way to "release" vtadmin-web is. Will need some discussion, so let's defer this out of v15.
For anyone running in k8s, this is not a problem since we do publish a vtadmin docker image.

@frouioui frouioui added this to v16.0.0 Nov 15, 2022
@frouioui frouioui moved this to Backlog in v16.0.0 Nov 15, 2022
@frouioui frouioui removed this from the v16.0 milestone Nov 15, 2022
@frouioui frouioui removed this from v16.0.0 Feb 8, 2023
@frouioui frouioui added this to v17.0.0 Feb 8, 2023
@frouioui frouioui moved this to Backlog in v17.0.0 Feb 8, 2023
@github-project-automation github-project-automation bot moved this from Backlog to Done in v17.0.0 Jun 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants