PLUMgrid charm - plumgrid-edge - review required

Thanks for your contributions to the Juju community.

Per review and discussion with James Page I am putting this bug into "In Progress" while the updated set of PLUMgrid charms is made. Please update the status of this bug to "Fix Committed" when a subsequent review is needed.

-thanks,
Antonio

Hey guys just to get you on the same page as us, let me share our progress.
The reviews pointed out the following problems

1. Charles pointed out that our charms were not in accordance with the juju charms policy. They are basic things like passing charms through charm proof. I have already made those changes and will be pushing them in the next patch.

2. Cory pointed out that our charms were failing on AWS. We had not been able to test our charms on AWS at the time therefore they failing on AWS. We have made the changes required and will the add them in the next patch.

3. James wants us to change the architecture of the charms. Currently our charms are dependent on each other but aren't subordinates therefore we have to ensure that they get deployed on the same node. We have finalized the architecture after discussions with James. Our discussion can be followed on the neutron-iovisor charm bug and also these drawings (https://docs.google.com/drawings/d/1_QVGe-uG-iioqeAtaCfQ6H4E5DAVQYAgD7rLAEkUTnU , https://docs.google.com/drawings/d/1rnsMMt4BaiL1qB2nZHKKYuzrhlhmBw6Iba5CH4OYn5s ). We are currently implementing the proposed changes which will take some time as the changes are significant.

Hopefully we will be able to update the charms in the next week.

Greetings Bilal,

since the last review there have been significant changes to the plumgrid-edge charm, and I'm pleased to review this charm again to see the improvements.

I do have some minor nitpicks that surfaced during the review and should be final wrap up fixes for this charm before acceptance:

It appears that when updating the README, `charm add readme` was used, and the .ex file was left. This triggers a warning in `charm proof`

W: Includes template README.ex file
W: README.ex includes line 6 of boilerplate README.ex

The README.ex file should be renamed to README.md to resolve when running charm proof. Additionally there is a single line in the README that is getting tripped up on `charm proof` - Step by step instructions on using the charm: I have proposed a merge against charm-tools to address this output making it much easier for a developer to know what has failed the linting routine here: https://code.launchpad.net/~lazypower/charm-tools/refactor-readme-lint/+merge/266784 so future issues coming from charm proof wrt the readme should be more helpful.

The lint target fails with the following output:

hooks/pg_edge_context.py:3:80: E501 line too long (82 > 79 characters)
hooks/pg_edge_context.py:54:80: E501 line too long (80 > 79 characters)
hooks/pg_edge_utils.py:115:80: E501 line too long (127 > 79 characters)
hooks/pg_edge_utils.py:150:80: E501 line too long (88 > 79 characters)
hooks/pg_edge_utils.py:206:80: E501 line too long (117 > 79 characters)
hooks/pg_edge_utils.py:222:80: E501 line too long (99 > 79 characters)
make: *** [lint] Error 1

when executing the plumgrid-edge tests, they failed due to not being able to find the `files/plumgrid-edge.yaml` file which is due to how the tests are being rund. Cory suggested a working fix for plumgrid-director: you should probably reference the file using: os.path.join(os.path.dirname(__file__), 'files/plumgrid-edge.yaml')

With these minor fixes I would have no problem approving the charm. Thanks so much for the refactoring and vigilance during the review process. I'm going to change status of this bug to "in progress" and when you're ready switch merge 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.

Bilal,

+1 LGTM. Thank you for the work you've put into this new submission for the charm store. Congrats on your achievement! I've promulgated your charm (placed it in the ~charmer recommended namespace) and you can find the bug tracker here: https://bugs.launchpad.net/charms/+source/plumgrid-edge and the project for your charm here: https://code.launchpad.net/~charmers/charms/trusty/plumgrid-edge/trunk

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.