Skip to content

Commit 2e90a77

Browse files
authored
Use OS aware runner instead of bash for run-local command; fixed loading requirements.txt on Windows (#2996)
## Changes Use OS aware runner instead of bash for run-local command ## Why `bash` might be not available on Windows and this also makes it consistent with other command executors like artifact building ## Tests Existing acceptance tests pass <!-- If your PR needs to be included in the release notes for next release, add a separate entry in NEXT_CHANGELOG.md as part of your PR. -->
1 parent a0a12cf commit 2e90a77

8 files changed

Lines changed: 52 additions & 23 deletions

File tree

NEXT_CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
### Dependency updates
88

99
### CLI
10+
* Use OS aware runner instead of bash for run-local command ([#2996](https://github.com/databricks/cli/pull/2996))
1011

1112
### Bundles
1213
* Fix "bundle summary -o json" to render null values properly ([#2990](https://github.com/databricks/cli/pull/2990))
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Flask==3.1.1

cmd/workspace/apps/run_local.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ func setupWorkspaceAndConfig(cmd *cobra.Command, entryPoint string, appPort int)
5353
func setupApp(cmd *cobra.Command, config *apps.Config, spec *apps.AppSpec, customEnv []string, prepareEnvironment bool) (apps.App, []string, error) {
5454
ctx := cmd.Context()
5555
cfg := cmdctx.ConfigUsed(ctx)
56-
app := apps.NewApp(config, spec)
56+
app := apps.NewApp(ctx, config, spec)
5757
env := auth.ProcessEnv(cfg)
5858
if cfg.Profile != "" {
5959
env = append(env, "DATABRICKS_CONFIG_PROFILE="+cfg.Profile)

libs/apps/apps.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,14 @@
11
package apps
22

3+
import "context"
4+
35
type App interface {
46
PrepareEnvironment() error
57
GetCommand(bool) ([]string, error)
68
}
79

8-
func NewApp(config *Config, spec *AppSpec) App {
10+
func NewApp(ctx context.Context, config *Config, spec *AppSpec) App {
911
// We only support python apps for now, but later we can add more types
1012
// based on AppSpec
11-
return NewPythonApp(config, spec)
13+
return NewPythonApp(ctx, config, spec)
1214
}

libs/apps/python.go

Lines changed: 23 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,15 @@
11
package apps
22

33
import (
4+
"context"
45
"errors"
56
"fmt"
67
"os"
7-
"os/exec"
88
"path/filepath"
99
"strconv"
1010
"strings"
11+
12+
"github.com/databricks/cli/libs/exec"
1113
)
1214

1315
const DEBUG_PORT = "5678"
@@ -37,16 +39,17 @@ var defaultLibraries = []string{
3739
}
3840

3941
type PythonApp struct {
42+
ctx context.Context
4043
config *Config
4144
spec *AppSpec
4245
uvArgs []string
4346
}
4447

45-
func NewPythonApp(config *Config, spec *AppSpec) *PythonApp {
48+
func NewPythonApp(ctx context.Context, config *Config, spec *AppSpec) *PythonApp {
4649
if config.DebugPort == "" {
4750
config.DebugPort = DEBUG_PORT
4851
}
49-
return &PythonApp{config: config, spec: spec}
52+
return &PythonApp{ctx: ctx, config: config, spec: spec}
5053
}
5154

5255
// PrepareEnvironment creates a Python virtual environment using uv and installs required dependencies.
@@ -68,7 +71,9 @@ func (p *PythonApp) PrepareEnvironment() error {
6871

6972
// Install requirements if they exist
7073
if _, err := os.Stat(filepath.Join(p.config.AppPath, "requirements.txt")); err == nil {
71-
reqArgs := []string{"uv", "pip", "install", "-r", filepath.Join(p.config.AppPath, "requirements.txt")}
74+
// We also execute command with CWD set at p.config.AppPath
75+
// so we can just path local path to requirements.txt here
76+
reqArgs := []string{"uv", "pip", "install", "-r", "requirements.txt"}
7277
if err := p.runCommand(reqArgs); err != nil {
7378
return err
7479
}
@@ -133,11 +138,19 @@ func (p *PythonApp) enableDebugging() {
133138
}
134139
}
135140

136-
// runCommand executes the given command as a bash command and returns any error.
141+
// runCommand executes the given command and returns any error.
137142
func (p *PythonApp) runCommand(args []string) error {
138-
cmd := exec.Command("bash", "-c", strings.Join(args, " "))
139-
cmd.Dir = p.spec.config.AppPath
140-
cmd.Stdout = os.Stdout
141-
cmd.Stderr = os.Stderr
142-
return cmd.Run()
143+
e, err := exec.NewCommandExecutor(p.config.AppPath)
144+
if err != nil {
145+
return err
146+
}
147+
e.WithInheritOutput()
148+
149+
// Safe to join args with spaces here since args are passed directly inside PrepareEnvironment() and GetCommand()
150+
// and don't contain user input.
151+
cmd, err := e.StartCommand(p.ctx, strings.Join(args, " "))
152+
if err != nil {
153+
return err
154+
}
155+
return cmd.Wait()
143156
}

libs/apps/python_test.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package apps
22

33
import (
4+
"context"
45
"path/filepath"
56
"testing"
67

@@ -88,7 +89,7 @@ func TestPythonAppGetCommand(t *testing.T) {
8889
for _, tt := range tests {
8990
t.Run(tt.name, func(t *testing.T) {
9091
config, spec := tt.setup()
91-
app := NewPythonApp(config, spec)
92+
app := NewPythonApp(context.Background(), config, spec)
9293
cmd, err := app.GetCommand(tt.debug)
9394

9495
if !tt.wantErr {

libs/exec/exec.go

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -61,21 +61,28 @@ func (c *command) Stderr() io.ReadCloser {
6161
}
6262

6363
type Executor struct {
64-
shell shell
65-
dir string
64+
shell shell
65+
dir string
66+
inheritOutput bool
6667
}
6768

69+
// NewCommandExecutor creates an Executor with default output behavior (no inheritance)
6870
func NewCommandExecutor(dir string) (*Executor, error) {
6971
shell, err := findShell()
7072
if err != nil {
7173
return nil, err
7274
}
7375
return &Executor{
74-
shell: shell,
75-
dir: dir,
76+
shell: shell,
77+
dir: dir,
78+
inheritOutput: false,
7679
}, nil
7780
}
7881

82+
func (e *Executor) WithInheritOutput() {
83+
e.inheritOutput = true
84+
}
85+
7986
func NewCommandExecutorWithExecutable(dir string, execType ExecutableType) (*Executor, error) {
8087
f, ok := finders[execType]
8188
if !ok {
@@ -107,20 +114,24 @@ func (e *Executor) StartCommand(ctx context.Context, command string) (Command, e
107114
if err != nil {
108115
return nil, err
109116
}
110-
return e.start(ctx, cmd, ec)
117+
return e.start(cmd, ec)
111118
}
112119

113-
func (e *Executor) start(ctx context.Context, cmd *osexec.Cmd, ec *execContext) (Command, error) {
120+
func (e *Executor) start(cmd *osexec.Cmd, ec *execContext) (Command, error) {
121+
if e.inheritOutput {
122+
cmd.Stdout = os.Stdout
123+
cmd.Stderr = os.Stderr
124+
cmd.Stdin = os.Stdin
125+
return &command{cmd, ec, nil, nil}, cmd.Start()
126+
}
114127
stdout, err := cmd.StdoutPipe()
115128
if err != nil {
116129
return nil, err
117130
}
118-
119131
stderr, err := cmd.StderrPipe()
120132
if err != nil {
121133
return nil, err
122134
}
123-
124135
return &command{cmd, ec, stdout, stderr}, cmd.Start()
125136
}
126137

libs/exec/exec_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ func TestExecutorCleanupsTempFiles(t *testing.T) {
123123
cmd, ec, err := executor.prepareCommand(context.Background(), "echo 'Hello'")
124124
assert.NoError(t, err)
125125

126-
command, err := executor.start(context.Background(), cmd, ec)
126+
command, err := executor.start(cmd, ec)
127127
assert.NoError(t, err)
128128

129129
fileName := ec.args[1]

0 commit comments

Comments
 (0)