cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Alena Prokharchyk <Alena.Prokharc...@citrix.com>
Subject Re: CLOUDSTACK-6242 - the fix needs to be reverted temporarily
Date Mon, 07 Jul 2014 21:05:57 GMT
Sure.

On 7/7/14, 2:01 PM, "Daan Hoogland" <daan.hoogland@gmail.com> wrote:

>Alena, I had no time at all to look at cloudstack today, could you
>apply it please. My $dayjob duties are requiring a slightly different
>focus the coming week(s)
>
>thanks
>
>On Mon, Jul 7, 2014 at 6:42 PM, Alena Prokharchyk
><Alena.Prokharchyk@citrix.com> wrote:
>> Yes, that¹s what I mean - by now, we should log them, but without
>>logging
>> the trace. So your fix is correct.
>>
>> Later, we should fix the installation process so the user is inserted
>>only
>> once; and change the logic in ConfigurationServer - instead of logging
>>the
>> exception, we should throw the RuntimeException indicating that
>>essential
>> CS configuration failed, and that exception should fail MS startup. I
>>will
>> file a bug for that after you submit your fix.
>>
>> Thank you,
>> Alena.
>>
>> On 7/3/14, 1:38 PM, "Daan Hoogland" <daan.hoogland@gmail.com> wrote:
>>
>>>Alena, I really want to fix issues in this line, because I really want
>>>us to use exceptions properly and never ignore them. So I would like
>>>handle them or log at least. Thanks for your patients.
>>>
>>>I am not sure of what you mean. Is this close:
>>>
>>>diff --git a/server/src/com/cloud/server/ConfigurationServerImpl.java
>>>b/server/src/com/cloud/server/ConfigurationServerImpl.java
>>>index b66e52d..a166372 100755
>>>--- a/server/src/com/cloud/server/ConfigurationServerImpl.java
>>>+++ b/server/src/com/cloud/server/ConfigurationServerImpl.java
>>>@@ -462,21 +462,21 @@
>>>
>>>                 try {
>>>                     PreparedStatement stmt =
>>>txn.prepareAutoCloseStatement(insertSql);
>>>                     stmt.executeUpdate();
>>>                 } catch (SQLException ex) {
>>>-                    s_logger.debug("Caught SQLException when
>>>inserting system account ", ex);
>>>+                    s_logger.debug("Caught SQLException when
>>>inserting system account: " + ex.getLocalizedMessage());
>>>                 }
>>>                 // insert system user
>>>                 insertSql = "INSERT INTO `cloud`.`user` (id, uuid,
>>>username, password, account_id, firstname, lastname, created,
>>>user.default)"
>>>                         + " VALUES (1, UUID(), 'system', RAND(), 1,
>>>'system', 'cloud', now(), 1)";
>>>
>>>                 try {
>>>                     PreparedStatement stmt =
>>>txn.prepareAutoCloseStatement(insertSql);
>>>                     stmt.executeUpdate();
>>>                 } catch (SQLException ex) {
>>>-                    s_logger.debug("Caught SQLException when
>>>inserting system user ", ex);
>>>+                    s_logger.debug("Caught SQLException when
>>>inserting system user: " + ex.getLocalizedMessage());
>>>                 }
>>>
>>>                 // insert admin user, but leave the account disabled
>>>until we set a
>>>                 // password with the user authenticator
>>>                 long id = 2;
>>>@@ -489,22 +489,22 @@
>>>                         + "', '1', '1', 1)";
>>>                 try {
>>>                     PreparedStatement stmt =
>>>txn.prepareAutoCloseStatement(insertSql);
>>>                     stmt.executeUpdate();
>>>                 } catch (SQLException ex) {
>>>-                    s_logger.debug("Caught SQLException when creating
>>>admin account ", ex);
>>>+                    s_logger.debug("Caught SQLException when creating
>>>admin account: " + ex.getLocalizedMessage());
>>>                 }
>>>
>>>                 // now insert the user
>>>                 insertSql = "INSERT INTO `cloud`.`user` (id, uuid,
>>>username, password, account_id, firstname, lastname, created, state,
>>>user.default) " + "VALUES (" + id
>>>                         + ", UUID(), '" + username + "', RAND(), 2,
>>>'" + firstname + "','" + lastname + "',now(), 'disabled', 1)";
>>>
>>>                 try {
>>>                     PreparedStatement stmt =
>>>txn.prepareAutoCloseStatement(insertSql);
>>>                     stmt.executeUpdate();
>>>                 } catch (SQLException ex) {
>>>-                    s_logger.debug("Caught SQLException when
>>>inserting user ", ex);
>>>+                    s_logger.debug("Caught SQLException when
>>>inserting user " + ex.getLocalizedMessage());
>>>                 }
>>>
>>>                 try {
>>>                     String tableName = "security_group";
>>>                     try {
>>>
>>>On Thu, Jul 3, 2014 at 10:23 PM, Alena Prokharchyk
>>><Alena.Prokharchyk@citrix.com> wrote:
>>>> Daan, there are similar problem in saveaccount/saveuser methods in thr
>>>>same class - introduced by this commit as well. I can fix it myself on
>>>>Monday (Its holiday days today and tomorrow at Citrix, usa); or you can
>>>>revert them as well along with the fix you do for network groups.
>>>>
>>>> Let me know, and thank you for the follow up.
>>>>
>>>> -alena
>>>>
>>>>> On Jul 2, 2014, at 11:43 PM, "Daan Hoogland"
>>>>><daan.hoogland@gmail.com>
>>>>>wrote:
>>>>>
>>>>> On Thu, Jul 3, 2014 at 12:27 AM, Alena Prokharchyk
>>>>> <Alena.Prokharchyk@citrix.com> wrote:
>>>>>> In any case, fixes done to ConfigurationManagerImpl are not correct,
>>>>>>and
>>>>>> logging should be fixed by reverting/reapplying the commit by
>>>>>>following
>>>>>> the rules defined in a) or b).
>>>>>
>>>>>
>>>>> changing to
>>>>>                     } catch (Exception ex) {
>>>>>                        // if network_groups table exists, create the
>>>>> default security group there
>>>>>                        s_logger.debug("Caught (SQL?)Exception: no
>>>>> network_group  " + ex.getLocalizedMessage());
>>>>>                    }
>>>>> for now
>>>>>
>>>>> --
>>>>> Daan
>>>
>>>
>>>
>>>--
>>>Daan
>>
>
>
>
>-- 
>Daan

Mime
View raw message