@wilderrodrigues @wido @bhaisaab @rajesh-battala @koushik-das @kishankavala @K0zka
I am taking this by exception type now so this is going to give conflicts pretty soon. I will continue on file by file basis to reduce the chances for that.

cloudstack-pull-rats #151 SUCCESS
This pull request looks good

cloudstack-pull-requests #849 SUCCESS
This pull request looks good

cloudstack-pull-rats #152 SUCCESS
This pull request looks good

cloudstack-pull-requests #850 SUCCESS
This pull request looks good

cloudstack-pull-analysis #84 SUCCESS
This pull request looks good

cloudstack-pull-analysis #85 SUCCESS
This pull request looks good

Since all these "ignore" messages are logged at INFO level, it needs to be ensured that logs are not getting filled up too soon. Also I see lot of instances for InterruptedException, I think they can they be excluded.

Hi @DaanHoogland

Nice you took time to have a look at those. It really helps sys admins to know what is going on with ACS. However, I would rather have them logged as WARN over INFO. Why? Because if we fall into the catch block, is because something went wrong or might probably require manual intervention. If you agree, would that be a lot of work to change them into WARN?

Concerning the logs getting filled up too soon, I wouldn't really bother. From my point of view that's an admin task to have a clever rotation of logs. If ACS code base was more robust, we could afford less logs and filling up files wouldn't be a problem. But since it isn't, let's make sure the users/admins are aware about what is happening.

With respect to the InterruptedExceptions, we can log them as debug, but ignoring them completely wouldn't be nice.

Cheers,
Wilder

@koushik-das, Ignoring interrupted exceptions means that you do not allow the user or other threads to interrupt you. To me that seems serious. I did not in any case see comment about why it was done so I rigorously added the messages.
As for level; On one hand you are right @wilderrodrigues , as some errors that have been ignored in the past have been problems. On the other hand, someone did think it was safe to ignore those errors. The real job is to asses all the messages on level but I regard that out of scope for this PR. Maybe some of them should be WARN or even ERROR and others merely DEBUG or TRACE. I put the tag [ignored] there so we know we must considder them for either proper handling or another level of logging.

Fair enough, @DaanHoogland

Thanks for the exaplanation. :)

LGTM 👍

@DaanHoogland
Agreed that setting appropriate log level is out of the scope for this PR. But we should file a jira ticket to track and address it before next release.

@kishankavala you actually made me think I should wait till after 4.6, on the other hand we will have to take the pain in a release.

Code wise it is LGTM

@kishankavala @DaanHoogland 👍 for the Jira ticket.

After 4.6 we can come back to this and assess the messages accordingly. But would be nice to have the logs in systems running 4.6.

Cheers,
Wilder

Changes LGTM, since we are creating a separate bug for correctly handling log levels.
@DaanHoogland I am not saying to remove the interrupted exceptions, just that logging messages for them may not be needed. But I agree with @wilderrodrigues that log level for these may be changed to DEBUG

So @koushik-das Do you propose the folowing?

• merge this for 4.6
• make a critical ticket for 4.7 to asses log levels
  note that I am not completely done yet and will have addressed all but the hyperv console before the end of the weekend hopefully.

@DaanHoogland Merge this in 4.6. If possible push the log level changes in 4.6 as well.

So I will amend the interrupted to be level debug and then merge. next I will create a new PR for any remaining empty catch blocks, @koushik-das.

cloudstack-pull-rats #163 SUCCESS
This pull request looks good

cloudstack-pull-requests #861 SUCCESS
This pull request looks good

cloudstack-pull-analysis #96 SUCCESS
This pull request looks good