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

Windows script files #950

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

bostko
Copy link
Contributor

@bostko bostko commented Oct 12, 2015

The PR includes #947 and it is
Dependent on cloudsoft/winrm4j#2

Introduces support for executing multiline commands from a script. This is needed so we can have more control over how commands are executed and catch properly their exit code.
Just like in ssh it uploads multiline commands into a script file on the machine and it is executing it from there.
Multiline commands into scripts is available only in WinRmMachineLocation. If we add it to AbstractSoftwareProcessWinRmDriver it will allow writing more complicated Windows Entities.

The most important part here is that I wrote Live tests for executing scripts so we can advice users better how to write Windows scripts properly.

import java.util.Collection;
import java.util.List;
import java.util.Map;
import java.util.Set;
Copy link
Contributor

Choose a reason for hiding this comment

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

Please configure your IDE to not reorder imports.

@bostko bostko force-pushed the windows_file_scripts branch 3 times, most recently from 36d8ed0 to 1fa3057 Compare October 15, 2015 14:31
@bostko bostko mentioned this pull request Oct 15, 2015
@bostko bostko force-pushed the windows_file_scripts branch 6 times, most recently from c4c3e9f to 71ba10a Compare October 16, 2015 17:25
@bostko bostko changed the title Windows file scripts Windows script files Oct 16, 2015
@ahgittin
Copy link
Contributor

@bostko any update on this? it looks like we are still waiting on cloudsoft/winrm4j#2 -- it looks like you addressed @aledsage 's remaining comment about @since there so maybe @rdowner or @alasdairhodge could take a quick look and merge that, cut a new release of that, then bump the version of that used in brooklyn (in this PR?) and then merge this?

@bostko bostko force-pushed the windows_file_scripts branch 2 times, most recently from acc2d97 to 7dad21b Compare October 28, 2015 14:01
@bostko
Copy link
Contributor Author

bostko commented Oct 28, 2015

@ahgittin Yes it is still waiting for cloudsoft/winrm4j#2
I've upgraded the winrm4j version in this PR as well.

@bostko
Copy link
Contributor Author

bostko commented Jan 12, 2016

Can you review it again? @aledsage
There could be some minor comments. I'll review those in the morning when I am fresh.

@bostko
Copy link
Contributor Author

bostko commented Jan 13, 2016

The current implementation of the code and WindowsScriptsWinrmExitStatusLiveTest the test is failing to instantiate runner in other words the driver which is using WinRmScriptTool.
Any ideas how to refactor it so I can have a live test for WinRmScriptTool?

@bostko
Copy link
Contributor Author

bostko commented Jan 13, 2016

Done.
VanillaWindowsProcessWinrmStreamsLiveTest, VanillaWindowsProcessWinrmStreamsLiveTest, WindowsScriptsWinrmExitStatusLiveTest are passing.
Can you review?

* @param commands
* @deprecated since 0.2; Use the {@link #executeCommand(String)} instead and transform your commands list explicitly
*/
@Deprecated
WinRmToolResponse executeScript(List<String> commands);
Copy link
Contributor

Choose a reason for hiding this comment

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

@bostko see @neykov 's comments in cloudsoft/winrm4j#2, saying "we should keep the (List<String>)...".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@aledsage I kept it. It is here

@bostko bostko force-pushed the windows_file_scripts branch 8 times, most recently from f9f96d0 to 8672510 Compare January 18, 2016 22:22
import java.util.Map;

public interface NativeWindowsScriptRunner {
public interface NaiveWindowsScriptRunner {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why rename this to "Naive" instead of "Native"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I named it similar to NaiveScriptRunner.
Docs added.

@aledsage
Copy link
Contributor

#1158 contains some of these same changes (it switches to using the pure-java winrm4j client v0.2.0).

@bostko bostko force-pushed the windows_file_scripts branch 2 times, most recently from fa7e0b8 to dec1f2c Compare January 31, 2016 21:12
Use executePsCommand and executeCmdCommand instead of executeScript

WinRM Script tests
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.

4 participants