-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8293/#review14751
-----------------------------------------------------------


Heya,

Good catch this one.

Please do not use the INSERT IGNORE syntax. This is Mysql specific and would 
prevent using any other database in the future. Isn't setting a random password 
a better solution?

Adding the logging statements is good.

Removing the NOT NULL requirements on the password field would lower (a very 
small) security threshold, which is something we shouldn't do.

Cheers,

Hugo

- Hugo Trippaers


On Nov. 30, 2012, 11:09 a.m., Hiroaki Kawai wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8293/
> -----------------------------------------------------------
> 
> (Updated Nov. 30, 2012, 11:09 a.m.)
> 
> 
> Review request for cloudstack.
> 
> 
> Description
> -------
> 
> Current com.cloud.server.ConfigurationServerImpl#saveUser tries to insert an 
> admin user entity into database, but the sql leaves password field 
> unspecified. This will always fail because the database schema declares 
> password field NOT NULL. This patch will fix database schema, and add missing 
> logging when this sql execution failed.
> 
> This happens only on current master:HEAD.
> 
> 
> Diffs
> -----
> 
>   server/src/com/cloud/server/ConfigurationServerImpl.java d2f0bb0 
>   setup/db/create-schema.sql fff084e 
> 
> Diff: https://reviews.apache.org/r/8293/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Hiroaki Kawai
> 
>

Reply via email to