Juju Charms Collection

Bug #1510672
Comment #1

Comment 1 for bug 1510672

Revision history for this message

Charles Butler (lazypower) wrote on 2015-11-13:

Greetings Narinder,

Thanks for the submission of the VSRG charm! I've taken some time to review the submission and I have the following notes:

The charm appears to fail automated testing during the install hook due to an apt repository issue. Relevant lines included below:

unit-nuage-vrsg-0[1935]: 2015-11-13 16:38:58 INFO unit.nuage-vrsg/0.install logger.go:40 W: Failed to fetch file:/var/lib/local-archive/VRS/dists/precise/main/binary-amd64/Packages File not found
unit-nuage-vrsg-0[1935]: 2015-11-13 16:38:58 INFO unit.nuage-vrsg/0.install logger.go:40 subprocess.CalledProcessError: Command '['apt-get', 'update']' returned non-zero exit status 100

Looking over the test, it seems very basic, I would like to see slightly more flexing of the installation - such as verifying open ports and other self-validation steps. Additional testing can be deferred to the final bundle such as verfying data sent over the wire to related services and the like.

Looking at the hook files I saw something interesting I haven't seen before, where there are 2 hooks decorated with @hooks.hook()

Without an implicit context defined, it seems that these methods would be executed on *every* hook invocation. Is this desired behavior? I dont think its inherently wrong, but it strikes me as odd that during a relationship exchange we want to inspect and possibly update apt repository settings.

Additionally there is an actions.py file in hooks/ that only performs a hookenv.log statement, this could be moved into hooks.py under the appropriate section thats calling it.

This is a good first round submission, I see a lot of good beginnings in here. With some additional work it will be ready for a charm store submission in no time.

Thanks again for the submission. I'm going to change status of this bug to "in progress" and when you're ready switch status to 'Fix Committed' and someone will be along shortly to review your work.

If you have any questions/comments/concerns about the review contact us in #juju on irc.freenode.net or email the mailing list <email address hidden>, or ask a question tagged with "juju" on http://askubuntu.com.

Greetings Narinder,

Thanks for the submission of the VSRG charm! I've taken some time to review the submission and I have the following notes:

The charm appears to fail automated testing during the install hook due to an apt repository issue. Relevant lines included below:

unit-nuage-vrsg-0[1935]: 2015-11-13 16:38:58 INFO unit.nuage-vrsg/0.install logger.go:40 W: Failed to fetch file:/var/lib/local-archive/VRS/dists/precise/main/binary-amd64/Packages  File not found
unit-nuage-vrsg-0[1935]: 2015-11-13 16:38:58 INFO unit.nuage-vrsg/0.install logger.go:40 subprocess.CalledProcessError: Command '['apt-get', 'update']' returned non-zero exit status 100

Looking at the hook files I saw something interesting I haven't seen before, where there are 2 hooks decorated with @hooks.hook()

Additionally there is an actions.py file in hooks/ that only performs a hookenv.log statement, this could be moved into hooks.py under the appropriate section thats calling it.

This is a good first round submission, I see a lot of good beginnings in here. With some additional work it will be ready for a charm store submission in no time.

Thanks again for the submission. I'm going to change status of this bug to "in progress" and when you're ready switch status to 'Fix Committed' and someone will be along shortly to review your work.

If you have any questions/comments/concerns about the review contact us in #juju on irc.freenode.net or email the mailing list juju@lists.ubuntu.com, or ask a question tagged with "juju" on http://askubuntu.com.