Juju Charms Collection
apache2 package

Merge lp://qastaging/~evarlast/charms/trusty/apache2/add-logs-interface into lp://qastaging/charms/trusty/apache2

  1. Trusty Tahr (14.04)
  2. add-logs-interface
  3. Merge into trunk
Proposed by Jay R. Wren
Status: Merged
Merged at revision: 68
Proposed branch: lp://qastaging/~evarlast/charms/trusty/apache2/add-logs-interface
Merge into: lp://qastaging/charms/trusty/apache2
Diff against target: 341 lines (+141/-24)
8 files modified
Makefile (+3/-3)
README.md (+6/-0)
hooks/hooks.py (+52/-0)
hooks/tests/test_cert.py (+12/-0)
hooks/tests/test_config_changed.py (+6/-2)
hooks/tests/test_log_relation.py (+56/-0)
hooks/tests/test_vhost_config_relation.py (+4/-19)
metadata.yaml (+2/-0)
To merge this branch: bzr merge lp://qastaging/~evarlast/charms/trusty/apache2/add-logs-interface
Reviewer Review Type Date Requested Status
Jay R. Wren (community) Needs Resubmitting
Andrew McLeod (community) Needs Fixing
Review Queue (community) automated testing Needs Fixing
charmers Pending
Review via email: mp+278222@code.qastaging.launchpad.net

Description of the change

Add logs relation.

This exposes log files written by apache via the logs relation. Logger services such as logstash can be related to this service and this will configure the logger to read these files.

To post a comment you must log in.
lp://qastaging/~evarlast/charms/trusty/apache2/add-logs-interface updated
70. By Jay R. Wren

merge parent

Revision history for this message
Review Queue (review-queue) wrote :

This item has failed automated testing! Results available here http://juju-ci.vapour.ws:8080/job/charm-bundle-test-lxc/1485/

review: Needs Fixing (automated testing)
Revision history for this message
Review Queue (review-queue) wrote :

This item has failed automated testing! Results available here http://juju-ci.vapour.ws:8080/job/charm-bundle-test-aws/1468/

review: Needs Fixing (automated testing)
lp://qastaging/~evarlast/charms/trusty/apache2/add-logs-interface updated
71. By Jay R. Wren

fix broken tests

Revision history for this message
Review Queue (review-queue) wrote :

This item has failed automated testing! Results available here http://juju-ci.vapour.ws:8080/job/charm-bundle-test-lxc/1535/

review: Needs Fixing (automated testing)
Revision history for this message
Review Queue (review-queue) wrote :

This item has failed automated testing! Results available here http://juju-ci.vapour.ws:8080/job/charm-bundle-test-aws/1518/

review: Needs Fixing (automated testing)
Revision history for this message
Andrew McLeod (admcleod) wrote :

Hi Jay,

As per my comments in your 'apt source' merge request, this one is failing with the following:

http://pastebin.ubuntu.com/13898257/

Andrew

review: Needs Fixing
lp://qastaging/~evarlast/charms/trusty/apache2/add-logs-interface updated
72. By Jay R. Wren

make utopic+ tests pass on trusty

Revision history for this message
Jay R. Wren (evarlast) wrote :

I don't see how these tests EVER passed this criteria. Evidence to the contrary is appreciated.

I've disabled the tests on trusty as the hook code in the charm doesn't work in trusty and is documented as such.

Please consider for merging again.

review: Needs Resubmitting
Revision history for this message
Cory Johns (johnsca) wrote :

Jay,

Thanks for addressing those test failures. I think your solution is reasonable, and the tests now pass for me. I think that this mostly looks good, except for two small items.

First, I think there's a corner case in how you write out to the logs relation, that I mentioned in my diff comment below. I think just dropping the call to relation_ids() entirely should resolve it without issue.

Second, I would like to see some documentation in the README about how this is supposed to be used. You mention logstash in the MR, above, but that charm doesn't support this relation directly. I'm guessing that you intend this to be used via the beaver charm (e.g., https://jujucharms.com/u/yellow/beaver/trusty/0) because that has an interface that seems to match up, but it's not at all clear.

With those two small changes, I would give this my +1.

lp://qastaging/~evarlast/charms/trusty/apache2/add-logs-interface updated
73. By Jay R. Wren

use relation_ids always, and add a section about logs relation to the README

Revision history for this message
Jay R. Wren (evarlast) wrote :

I have updated with those two things.

Revision history for this message
Cory Johns (johnsca) wrote :

Jay,

Thank you for addressing those items. This has now been merged, and should be ingested into the charm store within the hour.

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: