Re: Review Request 19917: Improvements on exception handlers (JIRA-6242)

2014-04-15 Thread daan Hoogland
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19917/#review40362 --- Ship it! Ship It! - daan Hoogland On April 5, 2014, 3:16 a.m.,

Re: Review Request 19917: Improvements on exception handlers (JIRA-6242)

2014-04-15 Thread daan Hoogland
On April 15, 2014, 8:28 a.m., daan Hoogland wrote: Ship It! c031eb7d38200d680da85ef57367b21df3483c41 - daan --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19917/#review40362

Re: Review Request 19917: Improvements on exception handlers (JIRA-6242)

2014-04-15 Thread Ding Yuan
HI Daan, Thanks a lot for committing it! Sorry for the troubles, and I will do what you said next time if I upload the patch (thanks for the suggestions)! Cheers! Ding On Apr 15, 2014, at 1:52 AM, Daan Hoogland daan.hoogl...@gmail.com wrote: it applies, I won't push it like this though. The

Re: Review Request 19917: Improvements on exception handlers (JIRA-6242)

2014-04-14 Thread daan Hoogland
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19917/#review40285 --- Ding, sorry for the late reaction. I forgot to look at your patch

Re: Review Request 19917: Improvements on exception handlers (JIRA-6242)

2014-04-14 Thread Ding Yuan
Hi Daan,Here you go! Please let me know if this is not what you want...Thanks a lot for this!Ding 0002-6242.patch Description: Binary data On Apr 14, 2014, at 2:13 PM, daan Hoogland daan.hoogl...@gmail.com wrote: This is an automatically generated e-mail. To reply,

Re: Review Request 19917: Improvements on exception handlers (JIRA-6242)

2014-04-14 Thread Daan Hoogland
thanks Ding, it doesn't apply, I'm afraid. Can you rebase it to latest master? Make sure it is only one commit. On Mon, Apr 14, 2014 at 8:48 PM, Ding Yuan y...@ece.utoronto.ca wrote: Hi Daan, Here you go! Please let me know if this is not what you want... Thanks a lot for this! Ding On

Re: Review Request 19917: Improvements on exception handlers (JIRA-6242)

2014-04-14 Thread Ding Yuan
Hi Daan,Sorry about that. Rebased my patch on the latest master. Attaching the patch. Please let me know if it still doesn’t work...thanks!Ding 0001-squash-6242-commits.patch Description: Binary data On Apr 14, 2014, at 3:04 PM, Daan Hoogland daan.hoogl...@gmail.com wrote:thanks Ding,it doesn't

Re: Review Request 19917: Improvements on exception handlers (JIRA-6242)

2014-04-14 Thread Daan Hoogland
it applies, I won't push it like this though. The comment says 'squash 6242 commits' so i will take the liberty of changing it to something like 'CLOUDSTACK-6242: exception handling improvements'. Also I'd appriciate it if you upload to reviewboard next time. This make reviewing easier for me.

Re: Review Request 19917: Improvements on exception handlers (JIRA-6242)

2014-04-06 Thread Ding Yuan
Hi Daan, Thanks for the clarification. I will try to do what you said. But it might be hard for me to trigger some of them because they’re in exception handling code. Still I will try. In the meantime, please let me know if there is anything I can further help from my side. Ding On Apr 5,

Re: Review Request 19917: Improvements on exception handlers (JIRA-6242)

2014-04-05 Thread Daan Hoogland
H Ding, I didn't mean for you to test it in a certain way. I was just curious about what you did with the patch before you submitted it. Did you start a cloud instance with it and try to hit any of the log messages for instance. You explained the background of your effort and I am curious as to

Re: Review Request 19917: Improvements on exception handlers (JIRA-6242)

2014-04-04 Thread Ding Yuan
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19917/ --- (Updated April 5, 2014, 3:16 a.m.) Review request for cloudstack, Alena

Re: Review Request 19917: Improvements on exception handlers (JIRA-6242)

2014-04-03 Thread Daan Hoogland
Ding, I think we can dare to do so in master as it will not see release for a while. We'll just have to be aware of the locations and be on alert for any stacktraces that will pass this list. I would not like to do this on the 4.4 branch even when it is an improvement of code quality as such. It

Re: Review Request 19917: Improvements on exception handlers (JIRA-6242)

2014-04-03 Thread Ding Yuan
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19917/ --- (Updated April 3, 2014, 3:22 p.m.) Review request for cloudstack. Changes

Re: Review Request 19917: Improvements on exception handlers (JIRA-6242)

2014-04-03 Thread Ding Yuan
Oops, sorry I didn’t publish my diff. I just published it on review board. Thanks for the comment Daan! Please let me know if I further need to improve it. Ding On Apr 3, 2014, at 9:52 AM, Daan Hoogland daan.hoogl...@gmail.com wrote: Ding, I think we can dare to do so in master as it will

Review Request 19917: Improvements on exception handlers (JIRA-6242)

2014-04-02 Thread Ding Yuan
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19917/ --- Review request for cloudstack. Repository: cloudstack-git Description

Re: Review Request 19917: Improvements on exception handlers (JIRA-6242)

2014-04-02 Thread daan Hoogland
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19917/#review39284 --- Ding Yuan, I like your work and explanation why it should be done.

Re: Review Request 19917: Improvements on exception handlers (JIRA-6242)

2014-04-02 Thread Ding Yuan
On April 2, 2014, 4:41 p.m., daan Hoogland wrote: Ding Yuan, I like your work and explanation why it should be done. I would like to see more distinct messages in the log statement. To often exactly the same line is logged from different locations, not indicating the reason for

Re: Review Request 19917: Improvements on exception handlers (JIRA-6242)

2014-04-02 Thread Laszlo Hornyak
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19917/#review39320 --- +1 nice work! - Laszlo Hornyak On April 2, 2014, 1:55 p.m., Ding

Re: Review Request 19917: Improvements on exception handlers (JIRA-6242)

2014-04-02 Thread Alena Prokharchyk
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19917/#review39324 --- Is there a reason why logs for some exceptions are being logged in

Re: Review Request 19917: Improvements on exception handlers (JIRA-6242)

2014-04-02 Thread Daan Hoogland
Alena, What I read in your comment is a description of INFO vs WARN. Debug would be only for outputting stacktraces to go with it or to indicate passing a certain code path. Agree? On Wed, Apr 2, 2014 at 8:31 PM, Alena Prokharchyk alena.prokharc...@citrix.com wrote:

Re: Review Request 19917: Improvements on exception handlers (JIRA-6242)

2014-04-02 Thread Alena Prokharchyk
Daan, Correct me if I¹m wrong, but all of the logging added by Ding, fall under to go with it or to indicate passing a certain code path². I¹ve just noticed that some of them were added with DEBUG, and some with WARN level, and wanted to correct that. So we should: 1) For sure: never print them

Re: Review Request 19917: Improvements on exception handlers (JIRA-6242)

2014-04-02 Thread Daan Hoogland
Ding Yuan, Any objections to that? On Wed, Apr 2, 2014 at 10:32 PM, Alena Prokharchyk alena.prokharc...@citrix.com wrote: On 4/2/14, 1:27 PM, Daan Hoogland daan.hoogl...@gmail.com wrote: I think we agree indeed. Doesn't mean we should start this discuss thread or write a arch guideline on

Re: Review Request 19917: Improvements on exception handlers (JIRA-6242)

2014-04-02 Thread Ding Yuan
Thanks all for the quick comments! If i understand the discussion correctly, I will just change all the added log printing statements to debug verbosity. I will upload a new patch for that shortly. Now a bit back story: the reason we are doing this is that we recently did an analysis on many

Re: Review Request 19917: Improvements on exception handlers (JIRA-6242)

2014-04-02 Thread Ding Yuan
Uploaded a new patch to 19917. Changed the verbosity to debug, and addressed Daan’s comment on providing more distinctive text messages. Sorry that I haven’t split them into smaller patches. Note in a few cases the original code was like: try { pstmt =