-
Notifications
You must be signed in to change notification settings - Fork 128
Appkit cli commands #4247
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
base: main
Are you sure you want to change the base?
Appkit cli commands #4247
Conversation
398f5f2 to
04f4c88
Compare
chore: fixup chore: fixup chore: fixup chore: fixup chore: fixup chore: fixup chore: fixup chore: fixup chore: fixup chore: fixup chore: fixup chore: fixup chore: fixup chore: fixup
04f4c88 to
6e23dec
Compare
|
Commit: f0b22d5
28 interesting tests: 17 RECOVERED, 6 KNOWN, 4 flaky, 1 SKIP
Top 48 slowest tests (at least 2 minutes):
|
| } | ||
|
|
||
| // downloadDirectory recursively downloads all files from a workspace path to a local directory. | ||
| func downloadDirectory(ctx context.Context, w *databricks.WorkspaceClient, remotePath, localDir string, force bool) (int, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should use generate.NewDownloader to re-use the existing file download code
| } | ||
|
|
||
| // Step 5: Run npm install if package.json exists | ||
| packageJSONPath := filepath.Join(destDir, "package.json") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should do another pass over the import command. I'd like to always create a bundle and then also make sure we bind the resources properly. That's something we could do in a follow up.
| <link rel="apple-touch-icon" sizes="180x180" href="/apple-touch-icon.png" /> | ||
| <link rel="manifest" href="/site.webmanifest" /> | ||
| <meta name="viewport" content="width=device-width, initial-scale=1.0" /> | ||
| <title>Created by Apps MCP</title> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| <title>Created by Apps MCP</title> | |
| <title>Created by Databricks AI Tools</title> |
experimental/apps-mcp/templates/appkit/generic/databricks.yml.tmpl
Outdated
Show resolved
Hide resolved
| "github.com/spf13/cobra" | ||
| ) | ||
|
|
||
| func New() *cobra.Command { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we drop the dev for now so iso databricks experimental dev apps ... we use databricks experimental apps ...?
It's already pretty long.
|
|
||
| // Step 1: Build frontend (unless skipped) | ||
| if !skipBuild { | ||
| if err := runNpmTypegen(ctx); err != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
running typegen make sense but only if it is a node.js app and we have a package.json
| log.Infof(ctx, "Running app: %s", appKey) | ||
| if err := runApp(ctx, b, appKey); err != nil { | ||
| cmdio.LogString(ctx, "✔ Deployment succeeded, but failed to start app") | ||
| return fmt.Errorf("failed to run app: %w", err) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should print a message telling the user to run databricks apps logs to debug this.
| var output syncBuffer | ||
|
|
||
| err := RunWithSpinnerCtx(ctx, "Generating types...", func() error { | ||
| cmd := exec.CommandContext(ctx, "npm", "run", "typegen") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| cmd := exec.CommandContext(ctx, "npm", "run", "typegen") | |
| cmd := exec.CommandContext(ctx, "npm", "run", "typegen", "--if-present") |
| var output syncBuffer | ||
|
|
||
| err := RunWithSpinnerCtx(ctx, "Building frontend...", func() error { | ||
| cmd := exec.CommandContext(ctx, "npm", "run", "build") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| cmd := exec.CommandContext(ctx, "npm", "run", "build") | |
| cmd := exec.CommandContext(ctx, "npm", "run", "build", "--if-present") |
| @@ -0,0 +1,419 @@ | |||
| package app | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
check with @pietern. He wants to unify CLI UX.
d552cd0 to
f0b22d5
Compare
Changes
Why
Tests