Code review comment for ~graylog-charmers/charm-graylog:feature/multi-version

Revision history for this message
Kevin W Monroe (kwmonroe) wrote :

I addressed previous comments in this last round of commits. Specifically:

- Guard install_graylog with @when_not(snap.installed.graylog). While I appreciate the idea of @when(core); @when_not(graylog), it's not needed for a couple reasons: (1) snap.install will ensure all prereqs are installed. (2) if 'core' ever changed names or graylog grew a new prereq, we'd have to come back and update the install_graylog decorators. Let's let snap handle the install just like we would apt.

- The beats input was being reconfigured every hook. There's no need for that. Instead, only reconfigure when config changes.

- From an earlier __init__.py comment, I decided to refactor the lib/charms/layer/graylog package. Move utils to a proper utils.py and make the relevant submodule bits known in __init__.py.

- From an earlier logextract.py comment, set the right URL depending on which version of graylog is installed. (Big Thanks for this heads up -- i didn't realize the URLs had changed)

- Test and Readme updates.

Interested parties can kick the tires with cs:~graylog-charmers/graylog-45, which has been released through candidate.

review: Approve

« Back to merge proposal