Andy, Thank you for your comments. I've tried to apply what you addressed and committed my module again. This module finds all openstack information using OpenStack APIs and database. Thank you.
Best regards, Young-Hyun On Wed, Jul 9, 2014 at 10:24 AM, Andy Kurth <[email protected]> wrote: > Thanks Young. Looks good! If I understand correctly, you are avoiding the > need to use the CLI or cpan module by interacting directly with OpenStack > via the REST API? > > It looks like the only commands you're running on the management node are > "nova" and "qemu-img" in _get_flavor_type. Would it be possible to > accomplish this via the API? I haven't traced through how your code works > too deeply, but was wondering if the following could be used: > http://docs.openstack.org/api/openstack > -compute/2/content/Flavors-d1e4180.html > > It would be wonderful if you can eliminate the need for these to be > executed. This would mean a pure API solution with nothing special needing > to be installed on the management node. > > If you do need to call these commands, instead of using qx and backticks > are used to run commands on the management node. Please change this to > use: > my ($exit_status, $output) = $self->mn_os->execute($command); > > Also, always, always, always make sure $output and anything else you try to > parse with a regex are defined first. This will avoid some nasty "Use of > uninitialized value in pattern match" errors which could potentially lead > to the entire process dying. > > The indentation looks great! :) There are a few places where the curly > bracket style could be modified. Just about all of the existing code > places opening brackets on the same line as the while/for statement such > as: > while ($loop > 0) { > -instead of- > while ($loop > 0) > { > > Please add a pod "=head2 subroutine_name ... =cut" heading for every > subroutine. This is helpful for others to read/understand your code. The > pod syntax can be a bit finicky. You can tell if it is formatted properly > by running "pod2text openstack.pm". > > Lastly (as mainly a reminder), we will need to incorporate all of the > database changes in vcl.sql and whatever method we use for the next release > to replace update-vcl.sql. I made a reminder comment here: > https://issues.apache.org/jira/browse/VCL-764 > > Regards, > Andy >
