Merge lp://staging/~marcoceppi/charms/precise/wordpress/trunk into lp://staging/charms/wordpress

Proposed by Marco Ceppi
Status: Merged
Approved by: Mark Mims
Approved revision: no longer in the source branch.
Merged at revision: 55
Proposed branch: lp://staging/~marcoceppi/charms/precise/wordpress/trunk
Merge into: lp://staging/charms/wordpress
Diff against target: 2597 lines (+2340/-56) (has conflicts)
24 files modified
README.md (+152/-0)
config.yaml (+26/-0)
copyright (+1/-1)
files/_debug/apc.php (+1364/-0)
files/_debug/info.php (+3/-0)
files/charm/nginx/etc_nginx_nginx.conf (+58/-0)
files/charm/nginx/etc_nginx_sites-enabled_loadbalancer (+51/-0)
files/charm/nginx/etc_nginx_sites-enabled_wordpress (+72/-0)
files/charm/php/php5-fpm_pool.d_www.conf (+41/-0)
files/charm/php/php5_conf.d_apc.ini (+13/-0)
hooks/config-changed (+54/-0)
hooks/db-relation-changed (+94/-46)
hooks/db-relation-departed (+5/-0)
hooks/install (+56/-1)
hooks/loadbalancer-relation-joined (+124/-0)
hooks/nfs-relation-changed (+28/-0)
hooks/nfs-relation-departed (+20/-0)
hooks/restart (+4/-0)
hooks/start (+3/-0)
hooks/stop (+5/-2)
hooks/upgrade-charm (+11/-0)
inc/common (+139/-0)
metadata.yaml (+12/-6)
revision (+4/-0)
Text conflict in hooks/db-relation-changed
Text conflict in metadata.yaml
Text conflict in revision
To merge this branch: bzr merge lp://staging/~marcoceppi/charms/precise/wordpress/trunk
Reviewer Review Type Date Requested Status
Mark Mims (community) Approve
charmers Pending
Benjamin Kerensa Pending
Review via email: mp+120559@code.staging.launchpad.net

This proposal supersedes a proposal from 2012-08-07.

Description of the change

There are a great many changes made to this charm, I will try to sum them up:

DO ALL THE THINGS THE OMGUBUNTU SITE DOES
Remove all the OMG! Ubuntu specific items
Allow for the use of NFS mounts to share data for lazy bloggers who are popular
Provide three configuration options for users: bare, single, optimized

To post a comment you must log in.
Revision history for this message
Benjamin Kerensa (bkerensa) wrote : Posted in a previous version of this proposal

Looks sane to me.

review: Approve
Revision history for this message
Mark Mims (mark-mims) wrote : Posted in a previous version of this proposal

Ok, here's a static review: This is an _example_ charm we'll use in charmschools and demos, so I'll have more stuff to say than when reviewing a normal charm :)

# things to change

- love the config and especially the config-changed hook... it's very simple, organized, and demoable! Please put explicit values into config.yaml... please don't make me guess if I should put `Optimized` or `optimized`

- so `charm proof` gives `I: relation shared-fs has no hooks`. I understand the desire to add a more general relation than nfs, but please remove this metadata until it's implemented to keep it clean and simple for people to read (demo charm)

- instead of `files/wordpress/wp-content`, please pull themes and plugins from external repos as needed... it's just extra cruft in the charm and it seems like it could grow _considerably_ and change at a faster/different rate than the charm itself. We want this to be best-practice for charms (demo charm). Perhaps we should discuss this one in channel with the gang or on the list to get a consensus on this, but I'd certainly prefer configurable external repos.

- the `stop` hook references omgbackup... that's great but make it generic please

- I'd love to see some of your `files/charm` contents be actual templates where more stuff is externalized via config. e.g., ports and stuff. Well, ok... after looking some more, some of this stuff will *need* to be parametrized... i.e., there're urls littering the static config files. Those need to be in `config.yaml`

# discussion/recommendations

- it's unclear what `deets` is and why that's not coming from upstream repos

- `upgrade-charm` hook should maybe provide a way to change nginx config too?

Thanks man... totally excited about getting this pimped-out version in the main charm! we need more like this.

review: Needs Fixing
Revision history for this message
Marco Ceppi (marcoceppi) wrote : Posted in a previous version of this proposal

* Explicit values added (and config-changed updated to convert incoming values to lower case)
* Shared-fs relation removed, that will be added once Gluster charm is ready
* External repo added, currently defaults to github:jujutools/wordpress-site will be moved to an LP branch at a later date
* All references to OMGUbuntu removed
  * A backup charm would be a great idea, for now it's just not in the charm
* deets has been moved to _debug and is now a config option
* upgrade-charm now re-runs the hooks/config-changed to make sure everything gets re-set up

Revision history for this message
Mark Mims (mark-mims) wrote :

awesome man... thanks!

debug is brilliant... we should perhaps add that as a "best practice" for charming... enable debug configs for the service itself if it supports it.

review: Approve
Revision history for this message
Mark Mims (mark-mims) wrote :

marco, merge away.

Please watch out for the conflicts.

55. By Marco Ceppi

* Nginx as a webserver
* Allow for scale-out with nginx
* Track Themes, Plugins, and other custom files in remote repo
* Cache levels are user-definable
* Share files between nodes with NFS
* Lots of performance enhancements

# Todo
* Add memcache support
* Wider support for remote files

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches

to all changes: