launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #09031
[Merge] lp:~rvb/maas/use-reverse into lp:maas
Raphaël Badin has proposed merging lp:~rvb/maas/use-reverse into lp:maas with lp:~rvb/maas/pxe-off-preseed as a prerequisite.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~rvb/maas/use-reverse/+merge/111240
This branch fixes a problem I spotted when I was working on the pre-req branch.
The metadata application should be hooked up (in src/maas/urls.py) like this:
urlpatterns = patterns('',
url(r'^', include('maasserver.urls')),
url(r'^metadata/', include('metadataserver.urls')),
)
Note the trailing slash at the end of '^metadata/'. Otherwise, reverse(...) does not return a correct path.
See https://docs.djangoproject.com/en/dev/topics/http/urls/#naming-url-patterns for details.
I refactored src/metadataserver/tests/test_api.py to use Django's reverse method everywhere. The main benefit is that now we exercise the name=... part in all the urls definitions in src/metadataserver/urls.py (augmented test coverage). Also, note that this whole branch is neutral in terms of added/removed lines.
--
https://code.launchpad.net/~rvb/maas/use-reverse/+merge/111240
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rvb/maas/use-reverse into lp:maas.
=== modified file 'src/maas/urls.py'
--- src/maas/urls.py 2012-04-16 10:00:51 +0000
+++ src/maas/urls.py 2012-06-20 14:51:33 +0000
@@ -23,7 +23,7 @@
urlpatterns = patterns('',
url(r'^', include('maasserver.urls')),
- url(r'^metadata', include('metadataserver.urls')),
+ url(r'^metadata/', include('metadataserver.urls')),
)
if settings.STATIC_LOCAL_SERVE:
=== modified file 'src/metadataserver/tests/test_api.py'
--- src/metadataserver/tests/test_api.py 2012-06-20 14:51:33 +0000
+++ src/metadataserver/tests/test_api.py 2012-06-20 14:51:33 +0000
@@ -18,6 +18,7 @@
from django.conf import settings
from django.core.exceptions import PermissionDenied
+from django.core.urlresolvers import reverse
from maasserver.enum import NODE_STATUS
from maasserver.exceptions import (
MAASAPINotFound,
@@ -128,33 +129,6 @@
token = NodeKey.objects.get_token_for_node(node)
return OAuthAuthenticatedClient(get_node_init_user(), token)
- def make_url(self, path):
- """Create an absolute URL for `path` on the metadata API.
-
- :param path: Path within the metadata API to access. Should start
- with a slash.
- :return: An absolute URL for the given path within the metadata
- service tree.
- """
- assert path.startswith('/'), "Give absolute metadata API path."
- # Root of the metadata API service.
- metadata_root = "/metadata"
- return metadata_root + path
-
- def get(self, path, client=None):
- """GET a resource from the metadata API.
-
- :param path: Path within the metadata API to access. Should start
- with a slash.
- :param token: If given, authenticate the request using this token.
- :type token: oauth.oauth.OAuthToken
- :return: A response to the GET request.
- :rtype: django.http.HttpResponse
- """
- if client is None:
- client = self.client
- return client.get(self.make_url(path))
-
def call_signal(self, client=None, version='latest', files={}, **kwargs):
"""Call the API's signal method.
@@ -177,37 +151,43 @@
for name, content in files.items():
params[name] = BytesIO(content)
params[name].name = name
- return client.post(self.make_url('/%s/' % version), params)
+ url = reverse('metadata-version', args=[version])
+ return client.post(url, params)
def test_no_anonymous_access(self):
- self.assertEqual(httplib.UNAUTHORIZED, self.get('/').status_code)
+ self.assertEqual(
+ httplib.UNAUTHORIZED,
+ self.client.get(reverse('metadata')).status_code)
def test_metadata_index_shows_latest(self):
client = self.make_node_client()
- self.assertIn('latest', self.get('/', client).content)
+ self.assertIn('latest', client.get(reverse('metadata')).content)
def test_metadata_index_shows_only_known_versions(self):
client = self.make_node_client()
- for item in self.get('/', client).content.splitlines():
+ for item in client.get(reverse('metadata')).content.splitlines():
check_version(item)
# The test is that we get here without exception.
pass
def test_version_index_shows_meta_data(self):
client = self.make_node_client()
- items = self.get('/latest/', client).content.splitlines()
+ url = reverse('metadata-version', args=['latest'])
+ items = client.get(url).content.splitlines()
self.assertIn('meta-data', items)
def test_version_index_does_not_show_user_data_if_not_available(self):
client = self.make_node_client()
- items = self.get('/latest/', client).content.splitlines()
+ url = reverse('metadata-version', args=['latest'])
+ items = client.get(url).content.splitlines()
self.assertNotIn('user-data', items)
def test_version_index_shows_user_data_if_available(self):
node = factory.make_node()
NodeUserData.objects.set_user_data(node, b"User data for node")
client = self.make_node_client(node)
- items = self.get('/latest/', client).content.splitlines()
+ url = reverse('metadata-version', args=['latest'])
+ items = client.get(url).content.splitlines()
self.assertIn('user-data', items)
def test_meta_data_view_lists_fields(self):
@@ -215,20 +195,23 @@
user, _ = factory.make_user_with_keys(n_keys=2, username='my-user')
node = factory.make_node(owner=user)
client = self.make_node_client(node=node)
- response = self.get('/latest/meta-data/', client)
+ url = reverse('metadata-meta-data', args=['latest', ''])
+ response = client.get(url)
self.assertIn('text/plain', response['Content-Type'])
self.assertItemsEqual(
MetaDataHandler.fields, response.content.split())
def test_meta_data_view_is_sorted(self):
client = self.make_node_client()
- response = self.get('/latest/meta-data/', client)
+ url = reverse('metadata-meta-data', args=['latest', ''])
+ response = client.get(url)
attributes = response.content.split()
self.assertEqual(sorted(attributes), attributes)
def test_meta_data_unknown_item_is_not_found(self):
client = self.make_node_client()
- response = self.get('/latest/meta-data/UNKNOWN-ITEM-HA-HA-HA', client)
+ url = reverse('metadata-meta-data', args=['latest', 'UNKNOWN-ITEM'])
+ response = client.get(url)
self.assertEqual(httplib.NOT_FOUND, response.status_code)
def test_get_attribute_producer_supports_all_fields(self):
@@ -239,7 +222,8 @@
def test_meta_data_local_hostname_returns_hostname(self):
hostname = factory.getRandomString()
client = self.make_node_client(factory.make_node(hostname=hostname))
- response = self.get('/latest/meta-data/local-hostname', client)
+ url = reverse('metadata-meta-data', args=['latest', 'local-hostname'])
+ response = client.get(url)
self.assertEqual(
(httplib.OK, hostname),
(response.status_code, response.content.decode('ascii')))
@@ -248,7 +232,8 @@
def test_meta_data_instance_id_returns_system_id(self):
node = factory.make_node()
client = self.make_node_client(node)
- response = self.get('/latest/meta-data/instance-id', client)
+ url = reverse('metadata-meta-data', args=['latest', 'instance-id'])
+ response = client.get(url)
self.assertEqual(
(httplib.OK, node.system_id),
(response.status_code, response.content.decode('ascii')))
@@ -259,39 +244,45 @@
node = factory.make_node()
NodeUserData.objects.set_user_data(node, data)
client = self.make_node_client(node)
- response = self.get('/latest/user-data', client)
+ response = client.get(reverse('metadata-user-data', args=['latest']))
self.assertEqual('application/octet-stream', response['Content-Type'])
self.assertIsInstance(response.content, str)
self.assertEqual(
(httplib.OK, data), (response.status_code, response.content))
def test_user_data_for_node_without_user_data_returns_not_found(self):
- response = self.get('/latest/user-data', self.make_node_client())
+ client = self.make_node_client()
+ response = client.get(reverse('metadata-user-data', args=['latest']))
self.assertEqual(httplib.NOT_FOUND, response.status_code)
def test_public_keys_not_listed_for_node_without_public_keys(self):
- response = self.get('/latest/meta-data/', self.make_node_client())
+ url = reverse('metadata-meta-data', args=['latest', ''])
+ client = self.make_node_client()
+ response = client.get(url)
self.assertNotIn(
'public-keys', response.content.decode('ascii').split('\n'))
def test_public_keys_listed_for_node_with_public_keys(self):
user, _ = factory.make_user_with_keys(n_keys=2, username='my-user')
node = factory.make_node(owner=user)
- response = self.get(
- '/latest/meta-data/', self.make_node_client(node=node))
+ url = reverse('metadata-meta-data', args=['latest', ''])
+ client = self.make_node_client(node=node)
+ response = client.get(url)
self.assertIn(
'public-keys', response.content.decode('ascii').split('\n'))
def test_public_keys_for_node_without_public_keys_returns_not_found(self):
- response = self.get(
- '/latest/meta-data/public-keys', self.make_node_client())
+ url = reverse('metadata-meta-data', args=['latest', 'public-keys'])
+ client = self.make_node_client()
+ response = client.get(url)
self.assertEqual(httplib.NOT_FOUND, response.status_code)
def test_public_keys_for_node_returns_list_of_keys(self):
user, _ = factory.make_user_with_keys(n_keys=2, username='my-user')
node = factory.make_node(owner=user)
- response = self.get(
- '/latest/meta-data/public-keys', self.make_node_client(node=node))
+ url = reverse('metadata-meta-data', args=['latest', 'public-keys'])
+ client = self.make_node_client(node=node)
+ response = client.get(url)
self.assertEqual(httplib.OK, response.status_code)
keys = SSHKey.objects.filter(user=user).values_list('key', flat=True)
expected_response = '\n'.join(keys)
@@ -320,7 +311,8 @@
def test_signaling_requires_status_code(self):
node = factory.make_node(status=NODE_STATUS.COMMISSIONING)
client = self.make_node_client(node=node)
- response = client.post(self.make_url('/latest/'), {'op': 'signal'})
+ url = reverse('metadata-version', args=['latest'])
+ response = client.post(url, {'op': 'signal'})
self.assertEqual(httplib.BAD_REQUEST, response.status_code)
def test_signaling_rejects_unknown_status_code(self):
@@ -505,8 +497,10 @@
def test_api_retrieves_node_metadata_by_mac(self):
mac = factory.make_mac_address()
- response = self.get(
- '/latest/by-mac/%s/meta-data/instance-id/' % mac.mac_address)
+ url = reverse(
+ 'metadata-meta-data-by-mac',
+ args=['latest', mac.mac_address, 'instance-id'])
+ response = self.client.get(url)
self.assertEqual(
(httplib.OK, mac.node.system_id),
(response.status_code, response.content))
@@ -515,7 +509,9 @@
mac = factory.make_mac_address()
user_data = factory.getRandomString().encode('ascii')
NodeUserData.objects.set_user_data(mac.node, user_data)
- response = self.get('/latest/by-mac/%s/user-data' % mac.mac_address)
+ url = reverse(
+ 'metadata-user-data-by-mac', args=['latest', mac.mac_address])
+ response = self.client.get(url)
self.assertEqual(
(httplib.OK, user_data),
(response.status_code, response.content))
@@ -523,29 +519,33 @@
def test_api_normally_disallows_anonymous_node_metadata_access(self):
self.patch(settings, 'ALLOW_ANONYMOUS_METADATA_ACCESS', False)
mac = factory.make_mac_address()
- response = self.get(
- '/latest/by-mac/%s/meta-data/instance-id/' % mac.mac_address)
+ url = reverse(
+ 'metadata-meta-data-by-mac',
+ args=['latest', mac.mac_address, 'instance-id'])
+ response = self.client.get(url)
self.assertEqual(httplib.FORBIDDEN, response.status_code)
def test_netboot_off(self):
node = factory.make_node(netboot=True)
client = self.make_node_client(node=node)
- response = client.post(
- self.make_url('/latest/'), {'op': 'netboot_off'})
+ url = reverse('metadata-version', args=['latest'])
+ response = client.post(url, {'op': 'netboot_off'})
node = reload_object(node)
self.assertFalse(node.netboot, response)
def test_netboot_on(self):
node = factory.make_node(netboot=False)
client = self.make_node_client(node=node)
- response = client.post(self.make_url('/latest/'), {'op': 'netboot_on'})
+ url = reverse('metadata-version', args=['latest'])
+ response = client.post(url, {'op': 'netboot_on'})
node = reload_object(node)
self.assertTrue(node.netboot, response)
def test_anonymous_netboot_off(self):
node = factory.make_node(netboot=True)
- anon_netboot_off_url = self.make_url(
- '/latest/%s/edit/' % node.system_id)
+ anon_netboot_off_url = reverse(
+ 'metadata-anon-node-edit', args=['latest',
+ node.system_id])
response = self.client.post(
anon_netboot_off_url, {'op': 'netboot_off'})
node = reload_object(node)
