Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 215
Use OS aware runner instead of bash for run-local command; fixed loading requirements.txt on Windows#2996
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
Uh oh!
There was an error while loading. Please reload this page.
Use OS aware runner instead of bash for run-local command; fixed loading requirements.txt on Windows #2996
Changes from all commits
92142a6f6c8211491401736ba29dba19aa4064b6458c57032ac90de12b9db1505cbffbFile filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| Flask==3.1.1 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,12 +1,14 @@ | ||
| package apps | ||
| import "context" | ||
| type App interface { | ||
| PrepareEnvironment() error | ||
| GetCommand(bool) ([]string, error) | ||
| } | ||
| func NewApp(config *Config, spec *AppSpec) App { | ||
| func NewApp(ctx context.Context, config *Config, spec *AppSpec) App { | ||
| // We only support python apps for now, but later we can add more types | ||
| // based on AppSpec | ||
| return NewPythonApp(config, spec) | ||
| return NewPythonApp(ctx, config, spec) | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,13 +1,15 @@ | ||
| package apps | ||
| import ( | ||
| "context" | ||
| "errors" | ||
| "fmt" | ||
| "os" | ||
| "os/exec" | ||
| "path/filepath" | ||
| "strconv" | ||
| "strings" | ||
| "github.com/databricks/cli/libs/exec" | ||
| ) | ||
| const DEBUG_PORT = "5678" | ||
| @@ -37,16 +39,17 @@ var defaultLibraries = []string{ | ||
| } | ||
| type PythonApp struct { | ||
| ctx context.Context | ||
| config *Config | ||
| spec *AppSpec | ||
| uvArgs []string | ||
| } | ||
| func NewPythonApp(config *Config, spec *AppSpec) *PythonApp { | ||
| func NewPythonApp(ctx context.Context, config *Config, spec *AppSpec) *PythonApp { | ||
| if config.DebugPort == "" { | ||
| config.DebugPort = DEBUG_PORT | ||
| } | ||
| return &PythonApp{config: config, spec: spec} | ||
| return &PythonApp{ctx: ctx, config: config, spec: spec} | ||
| } | ||
| // PrepareEnvironment creates a Python virtual environment using uv and installs required dependencies. | ||
| @@ -68,7 +71,9 @@ func (p *PythonApp) PrepareEnvironment() error { | ||
| // Install requirements if they exist | ||
| if _, err := os.Stat(filepath.Join(p.config.AppPath, "requirements.txt")); err == nil { | ||
| reqArgs := []string{"uv", "pip", "install", "-r", filepath.Join(p.config.AppPath, "requirements.txt")} | ||
| // We also execute command with CWD set at p.config.AppPath | ||
| // so we can just path local path to requirements.txt here | ||
| reqArgs := []string{"uv", "pip", "install", "-r", "requirements.txt"} | ||
| if err := p.runCommand(reqArgs); err != nil { | ||
| return err | ||
| } | ||
| @@ -133,11 +138,19 @@ func (p *PythonApp) enableDebugging() { | ||
| } | ||
| } | ||
| // runCommand executes the given command as a bash command and returns any error. | ||
| // runCommand executes the given command and returns any error. | ||
| func (p *PythonApp) runCommand(args []string) error { | ||
| cmd := exec.Command("bash", "-c", strings.Join(args, " ")) | ||
| cmd.Dir = p.spec.config.AppPath | ||
| cmd.Stdout = os.Stdout | ||
| cmd.Stderr = os.Stderr | ||
| return cmd.Run() | ||
| e, err := exec.NewCommandExecutor(p.config.AppPath) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| e.WithInheritOutput() | ||
| // Safe to join args with spaces here since args are passed directly inside PrepareEnvironment() and GetCommand() | ||
| // and don't contain user input. | ||
| cmd, err := e.StartCommand(p.ctx, strings.Join(args, " ")) | ||
andrewnester marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| if err != nil { | ||
| return err | ||
| } | ||
| return cmd.Wait() | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -61,21 +61,28 @@ func (c *command) Stderr() io.ReadCloser { | ||
| } | ||
| type Executor struct { | ||
| shell shell | ||
| dir string | ||
| shell shell | ||
| dir string | ||
| inheritOutput bool | ||
| } | ||
| // NewCommandExecutor creates an Executor with default output behavior (no inheritance) | ||
| func NewCommandExecutor(dir string) (*Executor, error) { | ||
| shell, err := findShell() | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| return &Executor{ | ||
| shell: shell, | ||
| dir: dir, | ||
| shell: shell, | ||
| dir: dir, | ||
| inheritOutput: false, | ||
| }, nil | ||
| } | ||
| func (e *Executor) WithInheritOutput() { | ||
| e.inheritOutput = true | ||
| } | ||
| func NewCommandExecutorWithExecutable(dir string, execType ExecutableType) (*Executor, error) { | ||
| f, ok := finders[execType] | ||
| if !ok { | ||
| @@ -107,20 +114,24 @@ func (e *Executor) StartCommand(ctx context.Context, command string) (Command, e | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| return e.start(ctx, cmd, ec) | ||
| return e.start(cmd, ec) | ||
| } | ||
| func (e *Executor) start(ctx context.Context, cmd *osexec.Cmd, ec *execContext) (Command, error) { | ||
| func (e *Executor) start(cmd *osexec.Cmd, ec *execContext) (Command, error) { | ||
| if e.inheritOutput { | ||
| cmd.Stdout = os.Stdout | ||
| cmd.Stderr = os.Stderr | ||
| cmd.Stdin = os.Stdin | ||
Contributor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Inherit output should not inherit the input. | ||
| return &command{cmd, ec, nil, nil}, cmd.Start() | ||
| } | ||
| stdout, err := cmd.StdoutPipe() | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| stderr, err := cmd.StderrPipe() | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| return &command{cmd, ec, stdout, stderr}, cmd.Start() | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.