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

STORM-4106 Fix Storm.ps1 stdout/stderr feedback in Powershell ISE #3740

Merged
merged 1 commit into from
Nov 11, 2024

Conversation

Scomocouk
Copy link

@Scomocouk Scomocouk commented Nov 9, 2024

What is the purpose of the change

Addresses STORM-4106. Stdout\Stderr feedback when launching Storm.ps1 in Powershell ISE now works with this change
Feedback from regular Powershell cmd prompt continues to work with this fix too.

How was the change tested

Open Powershell ISE and run Storm.ps1 to run a command (e..g show usage with ./storm1.ps1 -h)
Before the fix the Powershell ISE returns nothing output from Storm.ps1
With this fix the output Storm.ps1 is seen in Powershell ISE command window (and continues to be seen in regular Powershell cmd prompt)

See attached screenshots from before and after fix for both Powershell ISE and regular Powershell command prompts

Before Fix
STORM-4106-before-fix

After Fix
STORM-4106-after-fix

@reiabreu reiabreu requested review from jnioche and rzo1 November 10, 2024 10:43
Copy link
Contributor

@rzo1 rzo1 left a comment

Choose a reason for hiding this comment

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

Not a windows user but change looks ok (after checking on & and how python is now called directly)

@Scomocouk
Copy link
Author

Thanks @rzo1, incidentally, the & operator to call python3 directly is already being used in Storm.ps1 to check the version of python installed - see line 26:
$PythonVersion = (& python3 -V 2>&1).Split(" ")[1];

@rzo1 rzo1 merged commit 6aee95d into apache:master Nov 11, 2024
18 checks passed
@reiabreu
Copy link
Contributor

Apologies @Scomocouk @rzo1 , meant to review this one.
I'm also not a Windows user, but the change and especially the before and after test look good.

Cheers

Sign up for free to 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.

3 participants