search

dell / iDRAC-Redfish-Scripting

Python and PowerShell scripting for Dell EMC PowerEdge iDRAC REST API with DMTF Redfish
GNU General Public License v2.0
606 stars 279 forks source link

Improvements for Set-PowerControlREDFISH (Powershell) #106

Closed mrclschstr closed 4 years ago

mrclschstr commented 4 years ago

First of all: Thanks for your work! I recently used your Set-PowerControlREDFISH Powershell function and I just want to share some improvements.

I think you meant ErrorVariable instead of ErrorAction, right? https://github.com/dell/iDRAC-Redfish-Scripting/blob/cd8d91f092633481ba71ed1724a697d7dcbfea41/Redfish%20PowerShell/Set-PowerControlREDFISH/Set-PowerControlREDFISH.psm1#L110 https://github.com/dell/iDRAC-Redfish-Scripting/blob/cd8d91f092633481ba71ed1724a697d7dcbfea41/Redfish%20PowerShell/Set-PowerControlREDFISH/Set-PowerControlREDFISH.psm1#L115

Here you are missing the UseBasicParsing parameter you already used before. https://github.com/dell/iDRAC-Redfish-Scripting/blob/cd8d91f092633481ba71ed1724a697d7dcbfea41/Redfish%20PowerShell/Set-PowerControlREDFISH/Set-PowerControlREDFISH.psm1#L171 https://github.com/dell/iDRAC-Redfish-Scripting/blob/cd8d91f092633481ba71ed1724a697d7dcbfea41/Redfish%20PowerShell/Set-PowerControlREDFISH/Set-PowerControlREDFISH.psm1#L176

The break statements in the following lines aren't really helpful when you dot-source and use the Set-PowerControlREDFISH function within a loop. This breaks the loop and you cannot handle errors appropiately. https://github.com/dell/iDRAC-Redfish-Scripting/blob/cd8d91f092633481ba71ed1724a697d7dcbfea41/Redfish%20PowerShell/Set-PowerControlREDFISH/Set-PowerControlREDFISH.psm1#L122 https://github.com/dell/iDRAC-Redfish-Scripting/blob/cd8d91f092633481ba71ed1724a697d7dcbfea41/Redfish%20PowerShell/Set-PowerControlREDFISH/Set-PowerControlREDFISH.psm1#L183

texroemer commented 4 years ago

Hi @mrclschstr

Thanks for the feedback on improving the cmdlets, i have made the changes, new cmdlets uploaded.

For "-UseBasicParsing", this parameter is no longer needed and seems it has no effect on the cmdlet when used since all web requests use basic parsing only.

Thanks

mrclschstr commented 4 years ago

Sidenote: Up to Powershell 6 the parameter has an effect. Without it I wasn't able to use the function properly.

texroemer commented 4 years ago

I just tried on Powershell 5 and it worked for me (iDRAC9 4.10), removed -UseBasicParsing for GET request.

What error are you seeing and also what version of iDRAC are you using?

mrclschstr commented 4 years ago

I'm using Powershell 5.1 on a Windows Server 2008 R2 machine and want to connect to an iDRAC 9 (v3.36.36.36). The error message is the following:

The response content cannot be parsed because the Internet Explorer engine is not available, or Internet Explorer's first-launch configuration is not complete. Specify the UseBasicParsing parameter and try again.

texroemer commented 4 years ago

I rollbacked the iDRAC to 3.36, still don't see any issues with removing -UseBasicParsing for Redfish requests.

Seems to be issue with IE itself, found this post about it:

https://stackoverflow.com/questions/38005341/the-response-content-cannot-be-parsed-because-the-internet-explorer-engine-is-no

mrclschstr commented 4 years ago

That might be correct. But if adding -UseBasicParsing doesn't hurt since Powershell 6, you could just include it for your legacy software users 😉

texroemer commented 4 years ago

I updated all cmdlets with -UseBasicParsing for backward compatibility support.

mrclschstr commented 4 years ago

Awesome, thank you!