Landscape Charm

Merge lp://qastaging/~ahasenack/landscape-charm/service-names-1379133 into lp://qastaging/~landscape/landscape-charm/trunk

  1. service-names-1379133
  2. Merge into trunk
Proposed by Andreas Hasenack
Status: Merged
Approved by: Andreas Hasenack
Approved revision: 225
Merged at revision: 210
Proposed branch: lp://qastaging/~ahasenack/landscape-charm/service-names-1379133
Merge into: lp://qastaging/~landscape/landscape-charm/trunk
Diff against target: 270 lines (+118/-22)
3 files modified
hooks/hooks.py (+46/-9)
hooks/lib/util.py (+2/-2)
hooks/test_hooks.py (+70/-11)
To merge this branch: bzr merge lp://qastaging/~ahasenack/landscape-charm/service-names-1379133
Reviewer Review Type Date Requested Status
Chad Smith Approve
Benji York (community) Approve
Review via email: mp+238299@code.qastaging.launchpad.net

Commit message

Update the haproxy jinja variables in the apache vhost template files according to whatever service name haproxy was deployed as.

Description of the change

Update the haproxy jinja variables in the apache vhost template files according to whatever service name haproxy was deployed as.

The apache charm expects the jinja variables in the vhost template files to be named after the services it relates to. So if haproxy is deployed as "landscape-haproxy", the variable needs to contain "landscapehaproxy" (sic).

An additional complication is that jinja variables need to be proper python, and the apache charm chose to swallow the "-". So we need to apply the same transformation in our side.

To post a comment you must log in.
Revision history for this message
Benji York (benji) wrote :

This branch looks good. I had one regex simplification and a couple of style nitpicks, but once you consider those you're good to go.

review: Approve
lp://qastaging/~ahasenack/landscape-charm/service-names-1379133 updated
219. By Andreas Hasenack

Simplify regexp

220. By Andreas Hasenack

Other review comments.

Revision history for this message
Andreas Hasenack (ahasenack) :
Revision history for this message
Benji York (benji) wrote :

Inline comment about my memory of PEP 257 failing.

lp://qastaging/~ahasenack/landscape-charm/service-names-1379133 updated
221. By Andreas Hasenack

docstrings

Revision history for this message
Chad Smith (chad.smith) wrote :

[1] & [2] inline comments that are nits. I'd like to avoid truncating unacceptable characters in var names if we can replace them with underscores instead. Maybe the hyphens in service names were there for readability.

[3] Also can you run "make lint" target and sort the lint errors in this branch.

Revision history for this message
Andreas Hasenack (ahasenack) wrote :

Chad, I'm doing what the apache charm already does. If haproxy is deployed as "landscape-haproxy", the apache charm will be looking for a variable prefixed with "landscapehaproxy". They don't replace "-" with "_" :(

Revision history for this message
Chad Smith (chad.smith) :
Revision history for this message
Chad Smith (chad.smith) :
Revision history for this message
Chad Smith (chad.smith) wrote :

minor inline docstring comment.

I'll check a full deployment with a hyphenated service name to confirm but +1 otherwise.

review: Approve
Revision history for this message
Chad Smith (chad.smith) wrote :

validated both 400 errors on trunk and the fix. +1

review: Approve
lp://qastaging/~ahasenack/landscape-charm/service-names-1379133 updated
222. By Andreas Hasenack

lint

223. By Andreas Hasenack

docstrings

224. By Andreas Hasenack

- haproxy_service_name is now a mandatory argument
- docstrings

225. By Andreas Hasenack

test_no_haproxy_service_name_if_not_related_to_haproxy()

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