vishesh92 commented on PR #8547: URL: https://github.com/apache/cloudstack/pull/8547#issuecomment-1910385336
> -1 (binding) > > @vishesh92 this PR does not fix #8506. The issue and the thread it mentions is related to a generic solution, which was not even discussed. @GutoVeronezi I agree that this PR does not fix #8506. We need to revisit the timeouts for Commands on both the MS (#8506) & agents (#8011 - Agents don't even consider timeout which is set in the Commands). The main purpose of #8502 was to fix the failures we were seeing in e2e tests, but I introduced another bug (because of using 60ms instead of 60s for the script's timeout). In this PR, I wanted to fix the bug I introduced and address your concerns at least for the changes I did in my previous PR (#8502). Regarding externalizing timeouts for all commands, IMO we shouldn't try to fix it for now. I would like to do that but it would be a very big change and it will require more discussion and planning because the same `Command` can processed by different providers depending on an environment and the required timeout can be different depending on the scenario and the setup. I would like to get this PR merged for now since it's a blocker for unblock 4.19 release. Let me know what you think? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
