Juju Charms Collection

Bug #1432489
Comment #19

Comment 19 for bug 1432489

Revision history for this message

Matt Bruzek (mbruzek) wrote on 2015-06-05:

#19

Thanks for the update on the IBM MQ charm. I reviewed the latest version and here is what I found.

I was able to download the developer version of IBM MQ linked in the README, and the charm deployed with this binary. Consider editing the README file to include download links near the Usage section. The README should also contain a “## Known Limitations and Issues” section to address the limitations on the “local” provider (that it can not use the “sysctl” commands). I deployed this charm on the local provider, and the sysctl commands in the test directory will not pass tests on the “local” (LXC) provider (but you could use the same logic in the charm to prevent calling the sysctl commands in the tests).

The mixture of spaces and tabs in the hooks is inconsistent and does not line up on some editors. Although bash allows a mixture of spaces and tabs it is best practice to use one or the other. Please edit all the hooks and use either spaces or tabs so the code lines up correctly.

Should the MQ service be running as a result of a properly deployed charm? When deploying and accepting the license I would expect to see the MQ service set up and running. This does not seem to be the case here. Suggest creating and starting a queue after installing the software. I would have expected to see IBM MQ running after the charm is started. See comment #14.

Thank you for writing more tests. Looking at the code you only create a queue and start the queue. A good test for a queue is to write a message and read a message from the queue to make sure it works. The current tests failed to run because they are not executable! After adding the executable permission to 00-setup, and 10-bundles-test.py the test failed with this message: “TabError: inconsistent use of tabs and spaces in indentation”. Python requires that you use 4 spaces for code blocks. Please run flake8 or pep8 against the python test file and fix any issues those tools report. If you are interested in how we test, we use bundletester ( https://github.com/juju-solutions/bundletester ) which you can install on a system, and I use the following options “bundletester -F -e amazon -l DEBUG -v”. Please make sure the tests run and pass before the next review. The commands run in the tests "sysctl" will fail on "local" environments so use the same logic in the config-changed hook to prevent calling the commands if they can not be called.

Can the IBM MQ software be downloaded by the IBM Installation Manager? A colleague of mine suggested using Installation Manager sofware in the charm could eliminate the user having to download the IBM MQ software ahead of time. Please look into this option as it would make the charm so much easier for users to install.

Thanks again for the contribution of time and effort. I am going to put this bug back into “In Progress” while you work on the issues found in this review. If you want to discuss the review please join us in #juju on freenode, my user id is “mbruzek”. If you have any other questions or concerns please email <email address hidden> or email me directly.

Thanks for the update on the IBM MQ charm.  I reviewed the latest version and here is what I found.

I was able to download the developer version of IBM MQ linked in the README, and the charm deployed with this binary.  Consider editing the README file to include download links near the Usage section.  The README should also contain a “## Known Limitations and Issues” section to address the limitations on the “local” provider (that it can not use the “sysctl” commands).  I deployed this charm on the local provider, and the sysctl commands in the test directory will not pass tests on the “local” (LXC) provider (but you could use the same logic in the charm to prevent calling the sysctl commands in the tests).

The mixture of spaces and tabs in the hooks is inconsistent and does not line up on some editors.  Although bash allows a mixture of spaces and tabs it is best practice to use one or the other. Please edit all the hooks and use either spaces or tabs so the code lines up correctly.

Should the MQ service be running as a result of a properly deployed charm?  When deploying and accepting the license I would expect to see the MQ service set up and running.  This does not seem to be the case here.  Suggest creating and starting a queue after installing the software.  I would have expected to see IBM MQ running after the charm is started.  See comment #14.

Thank you for writing more tests.  Looking at the code you only create a queue and start the queue. A good test for a queue is to write a message and read a message from the queue to make sure it works.  The current tests failed to run because they are not executable!  After adding the executable permission to 00-setup, and 10-bundles-test.py the test failed with this message: “TabError: inconsistent use of tabs and spaces in indentation”.  Python requires that you use 4 spaces for code blocks.  Please run flake8 or pep8 against the python test file and fix any issues those tools report.  If you are interested in how we test, we use bundletester ( https://github.com/juju-solutions/bundletester ) which you can install on a system, and I use the following options “bundletester -F -e amazon -l DEBUG -v”.  Please make sure the tests run and pass before the next review.  The commands run in the tests "sysctl" will fail on "local" environments so use the same logic in the config-changed hook to prevent calling the commands if they can not be called.

Can the IBM MQ software be downloaded by the IBM Installation Manager?  A colleague of mine suggested using Installation Manager sofware in the charm could eliminate the user having to download the IBM MQ software ahead of time.  Please look into this option as it would make the charm so much easier for users to install.

Thanks again for the contribution of time and effort.  I am going to put this bug back into “In Progress” while you work on the issues found in this review.  If you want to discuss the review please join us in #juju on freenode, my user id is “mbruzek”.  If you have any other questions or concerns please email juju@lists.ubuntu.com or email me directly.