launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #14180
[Merge] lp:~rvb/maas/maas-remove-archive-pool into lp:maas
Raphaël Badin has proposed merging lp:~rvb/maas/maas-remove-archive-pool into lp:maas.
Commit message:
Get rid of the pool of archives to choose from for the archive settings (main_archive, ports_archive and cloud_images_archive).
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~rvb/maas/maas-remove-archive-pool/+merge/134070
This branch is a trade-off between having archives linked to series/architectures and having a list of the possible archives as a configurable setting: this branch removes the list of possible archives and turns the archive settings (main_archive, ports_archive and cloud_images_archive) into editable fields (as opposed to dropdowns with a list of possible archives).
The rationale behind that change (which is actually a simplification over the model we have right now) is that we're not sure we need to have a model for this (a table with the correspondance series/architecture -> archive url) but we don't want to store everything in the settings either as it would be something we would have to undo should we decide to go forward with the model described above. This branch simplifies the code so that the archive settings are now simple editable fields.
Also, having a pool of archives (a list of 'choices' in Django's terminology) stored in the settings allows a user to change that value through the API to an invalid value that would render the settings page potentially unusable.
--
https://code.launchpad.net/~rvb/maas/maas-remove-archive-pool/+merge/134070
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rvb/maas/maas-remove-archive-pool into lp:maas.
=== modified file 'src/maasserver/forms.py'
--- src/maasserver/forms.py 2012-11-09 17:09:07 +0000
+++ src/maasserver/forms.py 2012-11-13 10:45:25 +0000
@@ -16,7 +16,6 @@
"get_action_form",
"get_node_edit_form",
"get_node_create_form",
- "HostnameFormField",
"MACAddressForm",
"MAASAndNetworkForm",
"NodeGroupInterfaceForm",
@@ -47,9 +46,7 @@
PermissionDenied,
ValidationError,
)
-from django.core.validators import URLValidator
from django.forms import (
- CharField,
Form,
ModelForm,
)
@@ -628,39 +625,38 @@
error_messages={'invalid_choice': INVALID_DISTRO_SERIES_MESSAGE})
+url_error_msg = "Enter a valid url (e.g. http://host.example.com)."
+
+
class UbuntuForm(ConfigForm):
"""Settings page, Ubuntu section."""
default_distro_series = forms.ChoiceField(
choices=DISTRO_SERIES_CHOICES, required=False,
label="Default distro series used for deployment",
error_messages={'invalid_choice': INVALID_DISTRO_SERIES_MESSAGE})
-
- def __init__(self, *args, **kwargs):
- super(UbuntuForm, self).__init__(*args, **kwargs)
- # The archive fields must be added dynamically because their
- # 'choices' must be evaluated each time the form is instantiated.
- self.fields['main_archive'] = forms.ChoiceField(
- label="Main archive",
- choices=Config.objects.get_config('archive_choices'),
- help_text=(
- "Archive used by nodes to retrieve packages and by cluster "
- "controllers to retrieve boot images (Intel architectures)."
- ))
- self.fields['ports_archive'] = forms.ChoiceField(
- label="Ports archive",
- choices=Config.objects.get_config('archive_choices'),
- help_text=(
- "Archive used by cluster controllers to retrieve boot images "
- "(non-Intel architectures)."
- ))
- self.fields['cloud_images_archive'] = forms.ChoiceField(
- label="Cloud images archive",
- choices=Config.objects.get_config('archive_choices'),
- help_text=(
- "Archive used by the nodes to retrieve ephemeral images."
- ))
- # The list of fields has changed: load initial values.
- self._load_initials()
+ main_archive = forms.URLField(
+ label="Main archive",
+ error_messages={'invalid': url_error_msg},
+ help_text=(
+ "Archive used by nodes to retrieve packages and by cluster "
+ "controllers to retrieve boot images (Intel architectures). "
+ "E.g. http://archive.ubuntu.com/ubuntu.";
+ ))
+ ports_archive = forms.URLField(
+ label="Ports archive",
+ error_messages={'invalid': url_error_msg},
+ help_text=(
+ "Archive used by cluster controllers to retrieve boot images "
+ "(non-Intel architectures). "
+ "E.g. http://ports.ubuntu.com/ubuntu-ports.";
+ ))
+ cloud_images_archive = forms.URLField(
+ label="Cloud images archive",
+ error_messages={'invalid': url_error_msg},
+ help_text=(
+ "Archive used by the nodes to retrieve ephemeral images. "
+ "E.g. https://maas.ubuntu.com/images.";
+ ))
class GlobalKernelOptsForm(ConfigForm):
@@ -670,38 +666,6 @@
required=False)
-hostname_error_msg = "Enter a valid url (e.g. http://host.example.com)."
-
-
-def validate_url(value):
- try:
- validator = URLValidator(verify_exists=False)
- validator(value)
- except ValidationError:
- raise ValidationError(hostname_error_msg)
-
-
-class HostnameFormField(CharField):
-
- def __init__(self, *args, **kwargs):
- super(HostnameFormField, self).__init__(
- validators=[validate_url], *args, **kwargs)
-
-
-class AddArchiveForm(ConfigForm):
- archive_name = HostnameFormField(label="Archive name")
-
- def save(self):
- """Save the archive name in the Config table.
-
- This implementation of `save` does not support the `commit` argument.
- """
- archive_name = self.cleaned_data.get('archive_name')
- archives = Config.objects.get_config('archive_choices')
- archives.append([archive_name, archive_name])
- Config.objects.set_config('archive_choices', archives)
-
-
class NodeGroupInterfaceForm(ModelForm):
management = forms.TypedChoiceField(
=== modified file 'src/maasserver/models/config.py'
--- src/maasserver/models/config.py 2012-11-09 09:11:51 +0000
+++ src/maasserver/models/config.py 2012-11-13 10:45:25 +0000
@@ -45,22 +45,6 @@
'fallback_master_archive': False,
'keep_mirror_list_uptodate': False,
'fetch_new_releases': False,
- 'archive_choices': (
- [
- [
- 'http://archive.ubuntu.com/ubuntu',
- 'http://archive.ubuntu.com/ubuntu',
- ],
- [
- 'http://ports.ubuntu.com/ubuntu-ports',
- 'http://ports.ubuntu.com/ubuntu-ports',
- ],
- [
- 'https://maas.ubuntu.com/images',
- 'https://maas.ubuntu.com/images',
- ],
- ]
- ),
'main_archive': 'http://archive.ubuntu.com/ubuntu',
'ports_archive': 'http://ports.ubuntu.com/ubuntu-ports',
'cloud_images_archive': 'https://maas.ubuntu.com/images',
=== modified file 'src/maasserver/templates/maasserver/settings.html'
--- src/maasserver/templates/maasserver/settings.html 2012-11-09 09:11:51 +0000
+++ src/maasserver/templates/maasserver/settings.html 2012-11-13 10:45:25 +0000
@@ -95,25 +95,9 @@
<form action="{% url "settings" %}" method="post">
{% csrf_token %}
<ul>
- {% with field=ubuntu_form.default_distro_series %}
- {% include "maasserver/form_field.html" %}
- {% endwith %}
- {% with field=ubuntu_form.main_archive %}
- {% include "maasserver/form_field.html" %}
- {% endwith %}
- {% with field=ubuntu_form.ports_archive %}
- {% include "maasserver/form_field.html" %}
- {% endwith %}
- {% with field=ubuntu_form.cloud_images_archive %}
- {% include "maasserver/form_field.html" %}
- {% endwith %}
- <li>
- <a href="{% url "settings-add-archive" %}">
- <img src="{{ STATIC_URL }}img/inline_add.png"
- alt="Add" class="icon" />
- Add custom archive
- </a>
- </li>
+ {% for field in ubuntu_form %}
+ {% include "maasserver/form_field.html" %}
+ {% endfor %}
</ul>
<input type="hidden" name="ubuntu_submit" value="1" />
<input type="submit" class="button right" value="Save" />
=== modified file 'src/maasserver/tests/test_forms.py'
--- src/maasserver/tests/test_forms.py 2012-11-09 17:07:11 +0000
+++ src/maasserver/tests/test_forms.py 2012-11-13 10:45:25 +0000
@@ -16,10 +16,7 @@
from django import forms
from django.contrib.auth.models import User
-from django.core.exceptions import (
- PermissionDenied,
- ValidationError,
- )
+from django.core.exceptions import PermissionDenied
from django.http import QueryDict
from maasserver.enum import (
ARCHITECTURE,
@@ -37,7 +34,6 @@
get_action_form,
get_node_create_form,
get_node_edit_form,
- HostnameFormField,
initialize_node_group,
INTERFACES_VALIDATION_ERROR_MESSAGE,
MACAddressForm,
@@ -50,7 +46,6 @@
NodeWithMACAddressesForm,
ProfileForm,
remove_None_values,
- validate_url,
)
from maasserver.models import (
Config,
@@ -278,10 +273,6 @@
self.assertEqual(value, form.initial['field1'])
-class FormWithHostname(forms.Form):
- hostname = HostnameFormField()
-
-
class NodeEditForms(TestCase):
def test_NodeForm_contains_limited_set_of_fields(self):
@@ -513,37 +504,6 @@
form.save()
-class TestHostnameFormField(TestCase):
-
- def test_validate_url_validates_valid_hostnames(self):
- self.assertIsNone(validate_url('http://host.example.com'))
- self.assertIsNone(validate_url('http://host.my-example.com'))
- self.assertIsNone(validate_url('http://my-example.com'))
- # No ValidationError.
-
- def test_validate_url_does_not_validate_invalid_hostnames(self):
- self.assertRaises(ValidationError, validate_url, 'invalid-host')
-
- def test_validate_url_does_not_validate_too_long_hostnames(self):
- self.assertRaises(ValidationError, validate_url, 'toolong' * 100)
-
- def test_hostname_field_validation_cleaned_data_if_hostname_valid(self):
- form = FormWithHostname({'hostname': 'http://host.example.com'})
-
- self.assertTrue(form.is_valid())
- self.assertEqual(
- 'http://host.example.com', form.cleaned_data['hostname'])
-
- def test_hostname_field_validation_error_if_invalid_hostname(self):
- form = FormWithHostname({'hostname': 'invalid-host'})
-
- self.assertFalse(form.is_valid())
- self.assertItemsEqual(['hostname'], list(form.errors))
- self.assertEqual(
- ["Enter a valid url (e.g. http://host.example.com)."],
- form.errors['hostname'])
-
-
class TestUniqueEmailForms(TestCase):
def assertFormFailsValidationBecauseEmailNotUnique(self, form):
=== modified file 'src/maasserver/tests/test_views_settings.py'
--- src/maasserver/tests/test_views_settings.py 2012-11-09 09:11:51 +0000
+++ src/maasserver/tests/test_views_settings.py 2012-11-13 10:45:25 +0000
@@ -140,10 +140,9 @@
))
def test_settings_ubuntu_POST(self):
- choices = Config.objects.get_config('archive_choices')
- new_main_archive = factory.getRandomChoice(choices)
- new_ports_archive = factory.getRandomChoice(choices)
- new_cloud_images_archive = factory.getRandomChoice(choices)
+ new_main_archive = 'http://test.example.com/archive'
+ new_ports_archive = 'http://test2.example.com/archive'
+ new_cloud_images_archive = 'http://test3.example.com/archive'
new_default_distro_series = factory.getRandomEnum(DISTRO_SERIES)
response = self.client.post(
reverse('settings'),
@@ -171,19 +170,6 @@
Config.objects.get_config('default_distro_series'),
))
- def test_settings_add_archive_POST(self):
- choices = Config.objects.get_config('archive_choices')
- response = self.client.post(
- '/settings/archives/add/',
- data={'archive_name': 'http://my.hostname.com'}
- )
- new_choices = Config.objects.get_config('archive_choices')
-
- self.assertEqual(httplib.FOUND, response.status_code, response.content)
- self.assertItemsEqual(
- choices + [['http://my.hostname.com', 'http://my.hostname.com']],
- new_choices)
-
def test_settings_kernelopts_POST(self):
new_kernel_opts = "--new='arg' --flag=1 other"
response = self.client.post(
=== modified file 'src/maasserver/urls.py'
--- src/maasserver/urls.py 2012-10-15 09:32:06 +0000
+++ src/maasserver/urls.py 2012-11-13 10:45:25 +0000
@@ -46,7 +46,6 @@
AccountsEdit,
AccountsView,
settings,
- settings_add_archive,
)
from maasserver.views.settings_clusters import (
ClusterDelete,
@@ -157,9 +156,6 @@
'delete/$', ClusterInterfaceDelete.as_view()),
# /XXX
adminurl(r'^settings/$', settings, name='settings'),
- adminurl(
- r'^settings/archives/add/$', settings_add_archive,
- name='settings-add-archive'),
adminurl(r'^accounts/add/$', AccountsAdd.as_view(), name='accounts-add'),
adminurl(
r'^accounts/(?P<username>\w+)/edit/$', AccountsEdit.as_view(),
=== modified file 'src/maasserver/views/settings.py'
--- src/maasserver/views/settings.py 2012-11-08 10:40:48 +0000
+++ src/maasserver/views/settings.py 2012-11-13 10:45:25 +0000
@@ -16,7 +16,6 @@
"AccountsEdit",
"AccountsView",
"settings",
- "settings_add_archive",
]
from django.contrib import messages
@@ -40,13 +39,12 @@
from maasserver.enum import NODEGROUP_STATUS
from maasserver.exceptions import CannotDeleteUserException
from maasserver.forms import (
- AddArchiveForm,
CommissioningForm,
EditUserForm,
+ GlobalKernelOptsForm,
MAASAndNetworkForm,
NewUserCreationForm,
UbuntuForm,
- GlobalKernelOptsForm,
)
from maasserver.models import (
NodeGroup,
@@ -228,19 +226,3 @@
'kernelopts_form': kernelopts_form,
},
context_instance=RequestContext(request))
-
-
-def settings_add_archive(request):
- if request.method == 'POST':
- form = AddArchiveForm(request.POST)
- if form.is_valid():
- form.save()
- messages.info(request, "Archive added.")
- return HttpResponseRedirect(reverse('settings'))
- else:
- form = AddArchiveForm()
-
- return render_to_response(
- 'maasserver/settings_add_archive.html',
- {'form': form},
- context_instance=RequestContext(request))
