Merge lp:~gnuoy/python-jujuclient/ipv6 into lp:python-jujuclient

Proposed by Liam Young
Status: Merged
Merged at revision: 56
Proposed branch: lp:~gnuoy/python-jujuclient/ipv6
Merge into: lp:python-jujuclient
Diff against target: 64 lines (+33/-1)
2 files modified
jujuclient.py (+11/-1)
test_jujuclient.py (+22/-0)
To merge this branch: bzr merge lp:~gnuoy/python-jujuclient/ipv6
Reviewer Review Type Date Requested Status
Kapil Thangavelu Approve
Martin Packman (community) Approve
Review via email: mp+258770@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Martin Packman (gz) wrote :

Needs unit test. I'd add a new port-splitting function used here that can be validated with some ipv4 and ipv6 inputs.

review: Needs Fixing
lp:~gnuoy/python-jujuclient/ipv6 updated
57. By Liam Young

Add unit tests and move host port splitting into its own function

Revision history for this message
Martin Packman (gz) wrote :

Looks good. Python style nits inline.

review: Approve
Revision history for this message
Kapil Thangavelu (hazmat) wrote :

lgtm, with martin's style nits addressed.

review: Approve
lp:~gnuoy/python-jujuclient/ipv6 updated
58. By Liam Young

Tipdyup after mp feedback

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'jujuclient.py'
--- jujuclient.py 2015-02-27 12:03:22 +0000
+++ jujuclient.py 2015-05-11 14:34:07 +0000
@@ -19,6 +19,7 @@
19import logging19import logging
20import os20import os
21import pprint21import pprint
22import re
22import shutil23import shutil
23import signal24import signal
24import socket25import socket
@@ -218,9 +219,18 @@
218 data = yaml.safe_load(fh.read())219 data = yaml.safe_load(fh.read())
219 return jhome, data220 return jhome, data
220221
222 @staticmethod
223 def split_host_port(server):
224 m = re.match('(.*):(.*)', server)
225 if not m:
226 raise ValueError("Not an ipaddr/port {!r}".format(server))
227 address = m.group(1).strip("[]")
228 port = m.group(2)
229 return address, port
230
221 def is_server_available(self, server):231 def is_server_available(self, server):
222 """ Given address/port, return true/false if it's up """232 """ Given address/port, return true/false if it's up """
223 address, port = server.split(":")233 address, port = Connector.split_host_port(server)
224 try:234 try:
225 socket.create_connection((address, port), 3)235 socket.create_connection((address, port), 3)
226 return True236 return True
227237
=== modified file 'test_jujuclient.py'
--- test_jujuclient.py 2015-01-27 22:32:46 +0000
+++ test_jujuclient.py 2015-05-11 14:34:07 +0000
@@ -60,6 +60,28 @@
60 self.assertFalse(60 self.assertFalse(
61 Connector().is_server_available("foo.example.com:7070"))61 Connector().is_server_available("foo.example.com:7070"))
6262
63 def test_split_host_port_dns(self):
64 self.assertEqual(
65 Connector.split_host_port('foo.example.com:7070'),
66 ('foo.example.com', '7070')
67 )
68
69 def test_is_server_available_ipv4(self):
70 self.assertEqual(
71 Connector.split_host_port('10.0.0.10:7070'),
72 ('10.0.0.10', '7070')
73 )
74
75 def test_is_server_available_ipv6(self):
76 self.assertEqual(
77 Connector.split_host_port('[2001:db8::1]:7070'),
78 ('2001:db8::1', '7070')
79 )
80
81 def test_is_server_available_invalid_input(self):
82 self.assertRaises(
83 ValueError, Connector.split_host_port, 'I am not an ip/port combo'
84 )
6385
64class KeyManagerTest(unittest.TestCase):86class KeyManagerTest(unittest.TestCase):
65 def setUp(self):87 def setUp(self):

Subscribers

People subscribed via source and target branches

to all changes: