Charm Needed: CakePHP

[mr.haywood]

This bug is a formal request to add CakePHP to the list of charms available in Juju.
CakePHP lives here: http://cakephp.org/

Thanks in advance.

[Clint Byrum (clint-fewbar)]

Hi Mr. Haywood, thanks for your interest in juju.

CakePHP looks super awesome. If you have written a charm for it, please link to your branch and re-tag this bug with 'new-charm'. The tag is being removed right now though, as it is only meant to be used when there is a new-charm for the ~charmers team to review.

[Federico Gimenez (fgimenez)]

Ready for review. As instructed here [1] I've asked the Cake Software Foundation for permission to use the official logo in order to make a proper icon, still no response.

[1] http://cakephp.org/logos

Thanks, cheers
federico

[Jorge Niedbalski (niedbalski)]

Hello fgimenez,

Thanks for your good work on a new charm. I reviewed your charm code and I have a few observations to make it more useful to the community:

     - Is highly recommended to use the ubuntu archive versions instead of non-officially available packages.

      Fortunately the cakephp package is available un ubuntu universe: http://packages.ubuntu.com/trusty/cakephp,
      so a better idea would be to install this by using 'apt-get install cakephp'.

      If this version is too older for your purposes and you really need to pick the latest, please make sure that the version number is a
      declared variable inside the charm config.yaml file instead of a script scope local variable.

      - On this case would be also a pretty good idea to rewrite this charm for being a apache2 subordinate charm ( Please read : https://juju.ubuntu.com/docs/authors-subordinate-services.html ) , which is not mandatory but highly recommended
for a setup like this one.

Please don't hesistate in to ask any question.

[José Antonio Rey (jose)]

Hey, Federico!

Thanks for submitting CakePHP to the Charm Store! I have done a quick review of your submission, so let's get started!

 * `charm proof` returns no errors. Woohoo! There is, though, a warning, as the icon contains the template icon. It is fine to remove that file while you are working on getting permission from CakePHP.
 * As Jorge pointed out, it would be awesome if you could provide the user with two options, installing from the repository, or installing from source. Also, if installing from source, it'd be great if you could let the user choose what version of CakePHP they want to install.
 * When accessing CakePHP I get the following:
  * Notice (1024): Please change the value of 'Security.salt' in APP/Config/core.php to a salt value specific to your application. [CORE/Cake/Utility/Debugger.php, line 847]
  * Notice (1024): Please change the value of 'Security.cipherSeed' in APP/Config/core.php to a numeric (digits only) seed value specific to your application. [CORE/Cake/Utility/Debugger.php, line 851]
  * DebugKit is not installed. It will help you inspect and debug different aspects of your application. You can install it from GitHub
   It'd be nice if these can be fixed as they're upstream recommendation.

On the other hand, I think it is fine to have the charm as a non-subordinate charm. Make sure that if you put the repo/source/version options, the charm is idempotent, otherwise it will be breaking Charm Store policies.

If you need any help make sure to contact us at #juju on irc.freenode.net, or send us an email to <email address hidden>. We'll be happy to help you with the development of the charm. Thanks again for your efforts on the CakePHP charm!

[Federico Gimenez (fgimenez)]

Hey guys, thanks a lot for your comments. I'm already working on your suggestions and hope to have a fixed version soon.

Cheers,

[Federico Gimenez (fgimenez)]

Ready for review. I'm currently working on the apache2 subordinate version in another branch, I'll ping you when it will be ready.

Thx!

[José Antonio Rey (jose)]

Hey Federico,

Thanks again for submitting CakePHP to the Charm Store. I believe it would be an awesome addition to the store! I have now done a full review, and here's what I found.

# charm proof

Again, charm proof passes but for the icon warning (in which you are currently working, as far as I understand). Awesome first check!

# Deployment

I was able to successfully deploy CakePHP by following the instructions shown on the README.md file. That is perfect since users will be looking at it first-hand for instructions on how to deploy. There is just one small thing: when you deploy from source, it does not do the cryptographic verification of the file. I know that CakePHP does not provide SHA1SUMS for the different tarballs, so I would suggest hardcoding a series of them in a folder, in a file each, and maybe compare them.

# Configuration options

I started playing with both configuration options (installation_type and source_version) and both options seem to be idempotent. I changed them at random times, not necessarily at the same time, and it all worked and changes were reflected. Looks good here!

# Relations

At the current point, I see that the only relation is with mysql. I destroyed the relation with mysql, CakePHP showed an error because it couldn't connect to the DB. When I re-added it, everything was working alright. I went and destroyed my MySQL service and re-deployed, re-added the relation, and it was still working.

# Tests

The charm even has unit tests! I cannot run them at the current moment because I do not know how to modify them in order to deploy from the local branch, but will investigate further and let you know if I find any errors.

# Knitpicks

It would be nice to have 80-line wrapping in all metadata.yaml, config.yaml and README.md files to make it easier for people like me, reading on a Terminal.

Apart from that, I can not think of anything else at the current moment. As the cryptographic verification is not in place right now, I cannot give it a +1 and will have to mark it as Incomplete, but as soon as cryptographic verification is in there, I will make sure to take another look. I am really happy for how this charm is developing, and find it is pretty near inclusion on the Charm Store. Please, mark this charm as New or Fix Committed once you are ready for another review. As always, if you have any questions or need any help, make sure to send us an email to <email address hidden> or find us on #juju on irc.freenode.net - my nick is jose there.

[Federico Gimenez (fgimenez)]

Thanks Jose, ready for review. I've added some cryptographic files for recent versions of 2.5.x, 2.4.x and 1.3.x development branches. Now if the check fails the hook exits with error, and if there is no sha1sum file for the version being installed a warning is issued.

I've put a comment in the code stating that the sha1sum files should be added in sync with upstream versions. Being a task of the charm maintainers I thought that this is the best place, please let me know if this should be mentioned elsewhere.

I've also taken the opportunity to set the default version to 2.5.2.

Cheers

[José Antonio Rey (jose)]

Hey Federico!

I have now checked the charm and you have fixed what seemed to be the only critical error preventing me from giving it a +1. At the current time, the only things left are the knitpicks, which are not blockers. Again, that is the 80-line wrapping.

The charm is idempotent, the relations can be broken and re-build, and it deploys correctly.

From my side, it's a +1. Congratulations on this excellent piece of work.

A member of the ~charmers team should follow along in order to do another review and finalize the process if needed, or let you know if he finds any other errors that should be fixed beforehand. Again, thanks for your submission of the CakePHP charm to the Charm Store!

[Federico Gimenez (fgimenez)]

Thanks a lot for your comments Jose! Sorry, I focused on the cryptographic
checking and totally overlooked the 80-line wrapping, it's already fixed.

Cheers

[Matt Bruzek (mbruzek)]

Frederico,

Thank you for your time invested in the cakephp charm. I apologize for the delay since the last review.

I deployed the cakephp charm with both “package” and “source” configuration and it worked very well on my environments!

# Review

## charm proof:
W: No icon.svg file.
We would really like to see an icon file that makes the charms distinctive in the Charm Store. We provide a template as well as instructions on how to create an icon using Inkscape https://juju.ubuntu.com/docs/authors-charm-icon.html I see you have asked for permission to use an official icon, perhaps in the mean time you can use the instructions to create a generic icon for CakePHP.

## README.md:
The readme file is in markdown and converts to HTML correctly. The content looks great and has clear instructions on how to deploy! The links to the upstream project are very useful for me and other users that might not know much about CakePHP.

## hooks:
The bash hooks look great! I really like how you have documented what each function does and the input parameters with comments! This will help make the charm more maintainable in the future.

Apache2 data files: I believe it is convention for apache2 www files to be owned by www-data. You can do this by the following command:
sudo chown -R www-data:www-data /var/www/cakephp

## tests:
The CakePHP charm has amulet tests! I am very impressed and happy to have these tests. They will help automate our tests and raise the quality of charms in general. Thank you for taking the extra time to create the tests.

For the 02-package-install-type.py test I think you want to change subprocess.check_output(command) to self.sentry.unit['cakephp/0'].run(command) because you want to run the package check command on the remote unit not the computer running the test.

Thank you again for the submission of the cakephp charm! My review did not turn up any major problems, just those minor comments. The charm tooling will not allow me to promulgate a charm without an icon, and I see that you have asked for official approval. If you could create a generic CakePHP icon for the charm promulgation we could always put in another one when you get approval.

I am going to put this bug back to incomplete until the icon is added. Once you have added an icon let put the bug back in “Fix Committed” and we can promulgate the charm. Thanks for your continued work on this process, I really appreciate the time spent here!

[Federico Gimenez (fgimenez)]

Hi Matt, thanks a lot for your review and suggestions! I've uploaded a temporary icon following your advice, and also the two fixes you mention (files ownership and amulet tests should be fine now). Please, take a look and let me know what do you think.

Cheers!

[Matt Bruzek (mbruzek)]

Frederico,

Thank you for working on that icon, it looks great! The cakephp charm passes my review and has been pushed to the charm store. Congratulations on making it in the charm store!

Once the charm is injested you should be able to access the charm at: http://manage.jujucharms.com/charms/precise/cakephp

Since you already wrote tests, the cakphp charm would be an excellent candidate for a trusty (14.04) version that would require a separate merge proposal. Please be aware that the version of apache2 changed from precise to trusty and some default file names need to be updated in the version of apache2 that is in trusty (for example 000-default becomes 000-default.conf). Once you get everything working on trusty please submit a new branch for trusty.

Thanks again for all the work and your patience in the review process!