Code review comment for lp:~mbruzek/charms/precise/haproxy/trunk

Revision history for this message
Matt Bruzek (mbruzek) wrote :

Thanks for the review Benjamin.

https://codereview.appspot.com/56140043/diff/1/tests/10_deploy_test.py
File tests/10_deploy_test.py (right):

https://codereview.appspot.com/56140043/diff/1/tests/10_deploy_test.py#newcode19
tests/10_deploy_test.py:19: #d.add('apache2', units=units)
Thanks for the comment, removed the comments.

https://codereview.appspot.com/56140043/diff/1/tests/10_deploy_test.py#newcode24
tests/10_deploy_test.py:24: template_path = os.path.join(path,
'default_apache.tmpl')
I was not aware of pkg_resources. I get an error when I try that code in
python3.

Python 3.3.3 (default, Jan 11 2014, 05:48:01)
[GCC 4.8.2] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import pkg_resources
>>> template = pkg_resources.resource_string(__name__,
"default_apache.tmpl")
Traceback (most recent call last):
   File "<stdin>", line 1, in <module>
   File "/usr/lib/python3/dist-packages/pkg_resources.py", line 891, in
resource_string
     self, resource_name
   File "/usr/lib/python3/dist-packages/pkg_resources.py", line 1292, in
get_resource_string
     return self._get(self._fn(self.module_path, resource_name))
   File "/usr/lib/python3/dist-packages/pkg_resources.py", line 1370, in
_get
     "Can't perform this operation for loaders without 'get_data()'"
NotImplementedError: Can't perform this operation for loaders without
'get_data()'

Seems to work fine in python2.

https://codereview.appspot.com/56140043/diff/1/tests/10_deploy_test.py#newcode29
tests/10_deploy_test.py:29: configuration = {'vhost_https_template' :
encodedTemplate.decode('ascii')}
You are right I can convert (encode) the template with utf-8 but all the
character set of base65 can be represented in ascii, so I will keep that
on the decode.

https://codereview.appspot.com/56140043/diff/1/tests/10_deploy_test.py#newcode65
tests/10_deploy_test.py:65: print('Private address of the apache2
relation ', relation['private-address'])
I really like having the print there for manual testing. It does not
create too much extra logging I would prefer to keep it.

https://codereview.appspot.com/56140043/

« Back to merge proposal