Create 'service timeout' and 'appcmd timeout' command line parameters - #92
Conversation
|
@microsoft-github-policy-service agree @microsoft-github-policy-service agree company="Microsoft" |
|
Please can you review? Many thanks! |
| // Microsoft Visual C++ generated include file. | ||
| // Used by ServiceMonitor.rc | ||
| // | ||
| #define SM_MAJORNUMBER 2 |
There was a problem hiding this comment.
These version settings are already defined in version.h. We should be using the values from version.h so this file may not be needed.
| bool invalidSyntax = false; | ||
| int nextPositionalArgument = 1; | ||
|
|
||
| TCHAR buffDrive[3], buffDirName[1024], buffFileName[1024], buffExt[25]; |
There was a problem hiding this comment.
Please use wchar throughout.
| int index; | ||
|
|
||
| for (index = 0; index < wcslen(strpointer); index++) { | ||
| if ((strpointer[index] < 48) || (strpointer[index] > 57)) { |
There was a problem hiding this comment.
Please use iswdigit or '0'/'9' instead of harcoding ascii values.
| return TRUE; | ||
| } | ||
|
|
||
| bool isNumber(LPCTSTR strpointer) { |
| invalidSyntax = true; | ||
| break; | ||
| } else if (_wcsicmp(argv[argIndex], L"-st") == 0) { | ||
| if (isNumber(argv[argIndex + 1])) { |
There was a problem hiding this comment.
Should check that argIndex + 1 is less than argc.
|
We should track the .rc change to the copyright string separately from this PR since we would need to get it confirmed from a legal standpoint otherwise. |
18c25f5 to
d45f80f
Compare
|
Hi sujitnayak, Many thanks for your review. In light of your comments I have made some changes. Please forgive my coding...I have never coded in C before. If you want any more changes then please let me know and I'd be happy to do them. Kind regards, Lee |
| ~IISConfigUtil(); | ||
| HRESULT Initialize(); | ||
| HRESULT UpdateEnvironmentVarsToConfig(WCHAR* pstrAppPoolName); | ||
| HRESULT UpdateEnvironmentVarsToConfig(WCHAR* pstrAppPoolName, int appcmdTimeoutSeconds); |
There was a problem hiding this comment.
Could you please change the all the timeout types to DWORD?
Sorry for the delay in following up. The changes look good to me. I have 1 final comment about the type of the timeout variables. Once it is addressed, I will approve the PR. Thanks. |
Hi sujitnayak, No worries...thank you for taking the time to look at it. I have now changed the timeout variables from int to DWORD. Once again, any problems then please let me know. Out of interest, once this is approved (and merged) will the final .exe be automatically added to new Windows container images? Many thanks, Lee |
There is some additional work at our end to produce a signed binary that can be ingested into the container images. |
|
Hi sujitnayak, Is there any update on this issue? Will we see a signed release and will that release be added to the Windows containers? Many thanks and sorry to chase! Lee |
This PR fixes an issue whereby it fails intermittently for a number of my containers.
In my case, stopping the w3svc service took longer than the 20 seconds allowed by the original version. This version allows you to specify a timeout to restart the service.
I have also allowed an optional timeout to execute the appcmd command. This addresses an issue originally found here: #79
The new functionality added to this script does not break the existing behaviour.