Skip to content

Use OS aware runner instead of bash for run-local command; fixed loading requirements.txt on Windows - #2996

Merged
andrewnester merged 10 commits into
mainfrom
fix/run-local-windows-slash
Jun 6, 2025
Merged

Use OS aware runner instead of bash for run-local command; fixed loading requirements.txt on Windows#2996
andrewnester merged 10 commits into
mainfrom
fix/run-local-windows-slash

Conversation

@andrewnester

Copy link
Copy Markdown
Contributor

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

Comment threadlibs/apps/python.go Outdated
return err
}
_, err = e.Exec(p.ctx, strings.Join(args, " "))
return err

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This no longer forwards stdout/stderr.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Yes, addressed it here f6c8211

Wanted to tests which PR tests are actually failing on missing output

Comment threadlibs/apps/python.go
Comment threadlibs/apps/python.go Outdated
@andrewnester
andrewnester requested a review from denikJune 6, 2025 11:01
@andrewnesterandrewnester changed the title Use OS aware runner instead of bash for run-local commandUse OS aware runner instead of bash for run-local command; fixed loading requirements.txt on WindowsJun 6, 2025
@andrewnester
andrewnester enabled auto-merge June 6, 2025 11:03
@andrewnester
andrewnester added this pull request to the merge queueJun 6, 2025
Comment threadlibs/apps/python.go
Comment threadlibs/apps/python.go
Comment threadlibs/apps/python.go Outdated
@github-merge-queue
github-merge-queueBot removed this pull request from the merge queue due to failed status checks Jun 6, 2025
@andrewnester
andrewnester enabled auto-merge June 6, 2025 11:40
@andrewnester
andrewnester requested a review from pieternJune 6, 2025 11:40
@andrewnester
andrewnester added this pull request to the merge queueJun 6, 2025
Merged via the queue into main with commit 2e90a77Jun 6, 2025
@andrewnester
andrewnester deleted the fix/run-local-windows-slash branch June 6, 2025 12:10
Comment threadlibs/exec/exec.go
if e.inheritOutput {
cmd.Stdout = os.Stdout
cmd.Stderr = os.Stderr
cmd.Stdin = os.Stdin

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Inherit output should not inherit the input.

deco-sdk-taggingBot added a commit that referenced this pull request Jun 11, 2025
## Release v0.255.0
### Notable Changes
* Fix `databricks auth login` to tolerate URLs copied from the browser ([#3001](#3001)).
### CLI
* Use OS aware runner instead of bash for run-local command ([#2996](#2996))
### Bundles
* Fix "bundle summary -o json" to render null values properly ([#2990](#2990))
* Fix dashboard generation for already imported dashboard ([#3016](#3016))
* Fixed null pointer de-reference if artifacts missing fields ([#3022](#3022))
* Update bundle templates to also include `resources/*/*.yml` ([#3024](#3024))
* Apply YAML formatter on default-python and dbt-sql templates ([#3026](#3026))
denik pushed a commit that referenced this pull request May 20, 2026
…ing 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. -->
denik pushed a commit that referenced this pull request May 20, 2026
## Release v0.255.0
### Notable Changes
* Fix `databricks auth login` to tolerate URLs copied from the browser ([#3001](#3001)).
### CLI
* Use OS aware runner instead of bash for run-local command ([#2996](#2996))
### Bundles
* Fix "bundle summary -o json" to render null values properly ([#2990](#2990))
* Fix dashboard generation for already imported dashboard ([#3016](#3016))
* Fixed null pointer de-reference if artifacts missing fields ([#3022](#3022))
* Update bundle templates to also include `resources/*/*.yml` ([#3024](#3024))
* Apply YAML formatter on default-python and dbt-sql templates ([#3026](#3026))
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@andrewnester@pietern@denik@shreyas-goenka