Comment 7 for bug 1578273

Revision history for this message
Cory Johns (johnsca) wrote :

I was informed that the license is only applicable to the LSF Storage charm.

I was able to deploy and verify these charms according to the README. However, I noticed a few items in the code that I think should be addressed.

First, you are using the ibm-base layer but not any of the features it provides (it seems they've been supplanted by Juju 2.0 features such as terms and resources). When the resources are not attached properly, the layers from ibm-base, apt in particular, hide the status messages that you provide to the user and make it hard to tell why the charm is not working. I recommend replacing 'layer:ibm-base' with 'layer:basic' in layer.yaml. I tested with this and it works as expected and results in a much better user experience.

Finally, I noticed that you use "exit" in your bash reactive handlers. While this seems to work for the moment, it is not a good approach for reactive charms because it can cause handlers to be skipped in an unpredictable fashion, leading to hard to diagnose bugs. I believe your team has hit this before on other charms. From within a reactive handler (a function decorated with @when or the like), you can just replace "exit" with "return". For a function called by a reactive handler, it becomes a bit harder to surface up the fact that you wish to abort, but you can do something like:

    function foo() {
        return 1
    }

    @when 'state'
    function handler() {
        foo || return
    }