Charm Needed: Nginx

Bug #994699 reported by Brandon Holtsclaw
14
This bug affects 2 people
Affects Status Importance Assigned to Milestone
Juju Charms Collection
Incomplete
Medium
Brandon Holtsclaw

Bug Description

Nginx "Base" charm

Changed in charms:
assignee: nobody → Brandon Holtsclaw (imbrandon)
description: updated
Changed in charms:
assignee: Brandon Holtsclaw (imbrandon) → nobody
importance: Undecided → Medium
status: New → Fix Committed
Revision history for this message
Clint Byrum (clint-fewbar) wrote :

Hi Brandon. Thanks for submitting another full-featured charm. :)

Blockers:

* No maintainer -

$ charm proof
E: Charms need a maintainer (See RFC2822) - Name <email>

* NFS relation does nothing

I: relation nfs has no hooks

* readme doesn't explain how to actually use the charm

I really don't know what to do with this charm. Is it just for forking into other charms? I'm not sure what this charm gives me beyond 'apt-get install nginx'. Please explain this in readme.

Style questions:

* Using php, but then not using php?

run('mkdir -p /mnt/ramdisk/phpfpm-cache');
run('mkdir -p /mnt/ramdisk/tmp-cache');
run('mkdir -p /mnt/www');

Why are you using 'run' instead of just the 'mkdir' function?

* Wrong word in template

## placed by juju, do not edit or you will loose your changes

I suspect that should be 'will lose your changes'

* Using templates, but with static only values

$options = array('webroot');
$values = array('/mnt/www');
$default_site = template::replace($options,$values,template::read('default'));
template::write('/etc/nginx/sites-enabled/default',$default_site);

This seems.. fairly confusing. Whats the idea there?

* Using a non standard templating language

I'm not sure we should allow full implementations of templating in individual charms... heredocs or something established would make a lot more sense.

Setting to Incomplete until issues are addressed.

Changed in charms:
status: Fix Committed → Incomplete
assignee: nobody → Brandon Holtsclaw (imbrandon)
Revision history for this message
Brandon Holtsclaw (imbrandon) wrote :

Yea , more than fair to question 99% of that , as I knew this was not a complete charm as said before but needed a break from it a few days and an initial review to kick me back off....

ok just wanted to address a couple of points tho before I started jumping back in it, really minor just clarifications to your questions ( minus obvious omissions like maintainer.

Two main points I want to address as they will likely stay in the final charm unless more discussion is had and even then .... anyhow ok ....

>Style questions:
>* Using php, but then not using php?
>run('mkdir -p /mnt/ramdisk/phpfpm-cache');
>run('mkdir -p /mnt/ramdisk/tmp-cache');
>run('mkdir -p /mnt/www');
>Why are you using 'run' instead of just the 'mkdir' function?

I did it like this very deliberately and there is actually a 3rd pattern that you might have overlooked / not added together ( un-documented , but will be when done ) the reasoning of using backtic's as well to run some commands that again may on the surface seem lke using/not using but here is why and its consistent ... php "holds" the output when shelled out and needs to be examined/echo's back out to end up in the juju log, so if you'll notice any commands that may produce output are wrapped in a simple run function that runs the command then echo's as needed ( and can have some additional error checking added as well for exit codes later ) , thus that covers using the run() vs's built-ins for those things, then the backticks are used consistently where return data is NOT expected to goto juju-log and is fed directly to a variable for immediate use ...

ok now for the "templates" if you'll notice those are also very very simple wrapper functions for what equates to a sed replace ( using perl regex instead ) just like many if not every other charm out there, the only difference is i've taken the default config and for values i KNOW i need to replace over and over I put a token in place that is easily picked up my a very simple regex or sed to keep complexity to a minimum. if this charm where in bash i would do identically the same thing ... and as far as complexity I've seen far larger and more complex functions in quite a few charms already so i dont see what keeping them out of it does , other than adding to the workload NOW of putting it into a ppa package then installing it, when yes thats eventually what will happen for the bits that prove themselves but to be honest i dont think the cost / reward is worth doing it at this point for the reasons just given , not to say that it wont ever but working > perfect , especially when every single charm will need to be basically redone for some reason or another more than one time in the next 12 months.

sooooo overall yes i 100% agree with the very small thinks mentioned above ( even if long winded , still simple from both sides )
/me goes to make the changes ...

Revision history for this message
Eduard Gotwig (gotwig) wrote :

Not available on https://jujucharms.com/

Changed in charms:
status: Incomplete → Confirmed
Marco Ceppi (marcoceppi)
Changed in charms:
status: Confirmed → Incomplete
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.