Merge lp:~jtv/maas/bug-1372735 into lp:~maas-committers/maas/trunk
- bug-1372735
- Merge into trunk
Status: | Rejected | ||||
---|---|---|---|---|---|
Rejected by: | Jeroen T. Vermeulen | ||||
Proposed branch: | lp:~jtv/maas/bug-1372735 | ||||
Merge into: | lp:~maas-committers/maas/trunk | ||||
Prerequisite: | lp:~jtv/maas/bug-1372732 | ||||
Diff against target: |
567 lines (+88/-75) 15 files modified
src/maasserver/api/commissioning_scripts.py (+3/-3) src/maasserver/api/tests/test_nodegroup.py (+3/-3) src/maasserver/forms.py (+2/-2) src/maasserver/models/filestorage.py (+3/-3) src/maasserver/models/tests/test_node.py (+3/-3) src/maasserver/testing/factory.py (+4/-4) src/maasserver/tests/test_third_party_drivers.py (+2/-2) src/metadataserver/api.py (+4/-3) src/metadataserver/fields.py (+25/-16) src/metadataserver/migrations/0012_commission_result_binary_data_recode.py (+2/-3) src/metadataserver/models/commissioningscript.py (+3/-3) src/metadataserver/models/nodeuserdata.py (+4/-4) src/metadataserver/models/tests/test_nodecommissionresult.py (+3/-3) src/metadataserver/models/tests/test_noderesults.py (+2/-2) src/metadataserver/tests/test_fields.py (+25/-21) |
||||
To merge this branch: | bzr merge lp:~jtv/maas/bug-1372735 | ||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Gavin Panella (community) | Needs Information | ||
Review via email: mp+235559@code.launchpad.net |
This proposal supersedes a proposal from 2014-09-23.
Commit message
Fix DeprecationWarning in Bin class. It was showing up in Twisted logs, which in turn broke Node model tests sometimes (because they checked for the log to be empty).
The warning was about object.__init__ not wanting to receive arguments during the super() upcall from Bin.__init__. Even making just a direct upcall to bytes.__init__ (because Bin is derived from bytes, not from object directly) did not fix it. And so I had to replace use of the Bin constructor with a little factory.
Description of the change
I did want to keep the safety of checking against the wrong values being passed in, especially unicode or None. I don't see any way to check that _after_ construction, and I couldn't write my own constructor, so I had to get in _before_ construction.
Most of the diff will be pointless context around tiny mechanical changes in usage. It's actually quite a small change.
Jeroen
Newell Jensen (newell-jensen) : Posted in a previous version of this proposal | # |
Jeroen T. Vermeulen (jtv) wrote : | # |
Proposing your version as a fresh branch.
Preview Diff
1 | === modified file 'src/maasserver/api/commissioning_scripts.py' | |||
2 | --- src/maasserver/api/commissioning_scripts.py 2014-08-17 01:33:52 +0000 | |||
3 | +++ src/maasserver/api/commissioning_scripts.py 2014-09-23 04:43:27 +0000 | |||
4 | @@ -21,7 +21,7 @@ | |||
5 | 21 | from django.shortcuts import get_object_or_404 | 21 | from django.shortcuts import get_object_or_404 |
6 | 22 | from maasserver.api.support import OperationsHandler | 22 | from maasserver.api.support import OperationsHandler |
7 | 23 | from maasserver.api.utils import get_mandatory_param | 23 | from maasserver.api.utils import get_mandatory_param |
9 | 24 | from metadataserver.fields import Bin | 24 | from metadataserver.fields import wrap_bin |
10 | 25 | from metadataserver.models import CommissioningScript | 25 | from metadataserver.models import CommissioningScript |
11 | 26 | from piston.utils import rc | 26 | from piston.utils import rc |
12 | 27 | 27 | ||
13 | @@ -80,7 +80,7 @@ | |||
14 | 80 | is ignored; MAAS will know it by the name you pass to the request. | 80 | is ignored; MAAS will know it by the name you pass to the request. |
15 | 81 | """ | 81 | """ |
16 | 82 | name = get_mandatory_param(request.data, 'name') | 82 | name = get_mandatory_param(request.data, 'name') |
18 | 83 | content = Bin(get_content_parameter(request)) | 83 | content = wrap_bin(get_content_parameter(request)) |
19 | 84 | return CommissioningScript.objects.create(name=name, content=content) | 84 | return CommissioningScript.objects.create(name=name, content=content) |
20 | 85 | 85 | ||
21 | 86 | @classmethod | 86 | @classmethod |
22 | @@ -114,7 +114,7 @@ | |||
23 | 114 | 114 | ||
24 | 115 | def update(self, request, name): | 115 | def update(self, request, name): |
25 | 116 | """Update a commissioning script.""" | 116 | """Update a commissioning script.""" |
27 | 117 | content = Bin(get_content_parameter(request)) | 117 | content = wrap_bin(get_content_parameter(request)) |
28 | 118 | script = get_object_or_404(CommissioningScript, name=name) | 118 | script = get_object_or_404(CommissioningScript, name=name) |
29 | 119 | script.content = content | 119 | script.content = content |
30 | 120 | script.save() | 120 | script.save() |
31 | 121 | 121 | ||
32 | === modified file 'src/maasserver/api/tests/test_nodegroup.py' | |||
33 | --- src/maasserver/api/tests/test_nodegroup.py 2014-09-15 14:28:28 +0000 | |||
34 | +++ src/maasserver/api/tests/test_nodegroup.py 2014-09-23 04:43:27 +0000 | |||
35 | @@ -48,7 +48,7 @@ | |||
36 | 48 | from maastesting.celery import CeleryFixture | 48 | from maastesting.celery import CeleryFixture |
37 | 49 | from maastesting.matchers import MockCalledOnceWith | 49 | from maastesting.matchers import MockCalledOnceWith |
38 | 50 | from metadataserver.enum import RESULT_TYPE | 50 | from metadataserver.enum import RESULT_TYPE |
40 | 51 | from metadataserver.fields import Bin | 51 | from metadataserver.fields import wrap_bin |
41 | 52 | from metadataserver.models import ( | 52 | from metadataserver.models import ( |
42 | 53 | commissioningscript, | 53 | commissioningscript, |
43 | 54 | NodeResult, | 54 | NodeResult, |
44 | @@ -416,7 +416,7 @@ | |||
45 | 416 | NodeResult.objects.store_data( | 416 | NodeResult.objects.store_data( |
46 | 417 | node, commissioningscript.LSHW_OUTPUT_NAME, | 417 | node, commissioningscript.LSHW_OUTPUT_NAME, |
47 | 418 | script_result=0, result_type=RESULT_TYPE.COMMISSIONING, | 418 | script_result=0, result_type=RESULT_TYPE.COMMISSIONING, |
49 | 419 | data=Bin(data)) | 419 | data=wrap_bin(data)) |
50 | 420 | 420 | ||
51 | 421 | example_lldp_details = dedent("""\ | 421 | example_lldp_details = dedent("""\ |
52 | 422 | <?xml version="1.0" encoding="UTF-8"?> | 422 | <?xml version="1.0" encoding="UTF-8"?> |
53 | @@ -429,7 +429,7 @@ | |||
54 | 429 | NodeResult.objects.store_data( | 429 | NodeResult.objects.store_data( |
55 | 430 | node, commissioningscript.LLDP_OUTPUT_NAME, | 430 | node, commissioningscript.LLDP_OUTPUT_NAME, |
56 | 431 | script_result=0, result_type=RESULT_TYPE.COMMISSIONING, | 431 | script_result=0, result_type=RESULT_TYPE.COMMISSIONING, |
58 | 432 | data=Bin(data)) | 432 | data=wrap_bin(data)) |
59 | 433 | 433 | ||
60 | 434 | def test_nodegroup_requires_authentication(self): | 434 | def test_nodegroup_requires_authentication(self): |
61 | 435 | nodegroup = factory.make_NodeGroup() | 435 | nodegroup = factory.make_NodeGroup() |
62 | 436 | 436 | ||
63 | === modified file 'src/maasserver/forms.py' | |||
64 | --- src/maasserver/forms.py 2014-09-22 05:36:10 +0000 | |||
65 | +++ src/maasserver/forms.py 2014-09-23 04:43:27 +0000 | |||
66 | @@ -142,7 +142,7 @@ | |||
67 | 142 | list_osystem_choices, | 142 | list_osystem_choices, |
68 | 143 | list_release_choices, | 143 | list_release_choices, |
69 | 144 | ) | 144 | ) |
71 | 145 | from metadataserver.fields import Bin | 145 | from metadataserver.fields import wrap_bin |
72 | 146 | from metadataserver.models import CommissioningScript | 146 | from metadataserver.models import CommissioningScript |
73 | 147 | from netaddr import IPAddress | 147 | from netaddr import IPAddress |
74 | 148 | from provisioningserver.drivers.osystem import OperatingSystemRegistry | 148 | from provisioningserver.drivers.osystem import OperatingSystemRegistry |
75 | @@ -1620,7 +1620,7 @@ | |||
76 | 1620 | content = self.cleaned_data['content'] | 1620 | content = self.cleaned_data['content'] |
77 | 1621 | CommissioningScript.objects.create( | 1621 | CommissioningScript.objects.create( |
78 | 1622 | name=content.name, | 1622 | name=content.name, |
80 | 1623 | content=Bin(content.read())) | 1623 | content=wrap_bin(content.read())) |
81 | 1624 | 1624 | ||
82 | 1625 | 1625 | ||
83 | 1626 | class UnconstrainedMultipleChoiceField(MultipleChoiceField): | 1626 | class UnconstrainedMultipleChoiceField(MultipleChoiceField): |
84 | 1627 | 1627 | ||
85 | === modified file 'src/maasserver/models/filestorage.py' | |||
86 | --- src/maasserver/models/filestorage.py 2013-10-07 09:12:40 +0000 | |||
87 | +++ src/maasserver/models/filestorage.py 2014-09-23 04:43:27 +0000 | |||
88 | @@ -1,4 +1,4 @@ | |||
90 | 1 | # Copyright 2012 Canonical Ltd. This software is licensed under the | 1 | # Copyright 2012-2014 Canonical Ltd. This software is licensed under the |
91 | 2 | # GNU Affero General Public License version 3 (see the file LICENSE). | 2 | # GNU Affero General Public License version 3 (see the file LICENSE). |
92 | 3 | 3 | ||
93 | 4 | """Storage for uploaded files.""" | 4 | """Storage for uploaded files.""" |
94 | @@ -31,8 +31,8 @@ | |||
95 | 31 | from maasserver import DefaultMeta | 31 | from maasserver import DefaultMeta |
96 | 32 | from maasserver.models.cleansave import CleanSave | 32 | from maasserver.models.cleansave import CleanSave |
97 | 33 | from metadataserver.fields import ( | 33 | from metadataserver.fields import ( |
98 | 34 | Bin, | ||
99 | 35 | BinaryField, | 34 | BinaryField, |
100 | 35 | wrap_bin, | ||
101 | 36 | ) | 36 | ) |
102 | 37 | 37 | ||
103 | 38 | 38 | ||
104 | @@ -59,7 +59,7 @@ | |||
105 | 59 | """ | 59 | """ |
106 | 60 | # This probably ought to read in chunks but large files are | 60 | # This probably ought to read in chunks but large files are |
107 | 61 | # not expected. | 61 | # not expected. |
109 | 62 | content = Bin(file_object.read()) | 62 | content = wrap_bin(file_object.read()) |
110 | 63 | storage, created = self.get_or_create( | 63 | storage, created = self.get_or_create( |
111 | 64 | filename=filename, owner=owner, defaults={'content': content}) | 64 | filename=filename, owner=owner, defaults={'content': content}) |
112 | 65 | if not created: | 65 | if not created: |
113 | 66 | 66 | ||
114 | === modified file 'src/maasserver/models/tests/test_node.py' | |||
115 | --- src/maasserver/models/tests/test_node.py 2014-09-22 06:29:06 +0000 | |||
116 | +++ src/maasserver/models/tests/test_node.py 2014-09-23 04:43:27 +0000 | |||
117 | @@ -78,7 +78,7 @@ | |||
118 | 78 | from maastesting.testcase import MAASTestCase | 78 | from maastesting.testcase import MAASTestCase |
119 | 79 | from metadataserver import commissioning | 79 | from metadataserver import commissioning |
120 | 80 | from metadataserver.enum import RESULT_TYPE | 80 | from metadataserver.enum import RESULT_TYPE |
122 | 81 | from metadataserver.fields import Bin | 81 | from metadataserver.fields import wrap_bin |
123 | 82 | from metadataserver.models import ( | 82 | from metadataserver.models import ( |
124 | 83 | NodeResult, | 83 | NodeResult, |
125 | 84 | NodeUserData, | 84 | NodeUserData, |
126 | @@ -1050,7 +1050,7 @@ | |||
127 | 1050 | node, factory.make_string(), | 1050 | node, factory.make_string(), |
128 | 1051 | random.randint(0, 10), | 1051 | random.randint(0, 10), |
129 | 1052 | RESULT_TYPE.COMMISSIONING, | 1052 | RESULT_TYPE.COMMISSIONING, |
131 | 1053 | Bin(factory.make_bytes())) | 1053 | wrap_bin(factory.make_bytes())) |
132 | 1054 | node.start_commissioning(factory.make_admin()) | 1054 | node.start_commissioning(factory.make_admin()) |
133 | 1055 | self.assertItemsEqual([], node.noderesult_set.all()) | 1055 | self.assertItemsEqual([], node.noderesult_set.all()) |
134 | 1056 | 1056 | ||
135 | @@ -1061,7 +1061,7 @@ | |||
136 | 1061 | script_result = random.randint(0, 10) | 1061 | script_result = random.randint(0, 10) |
137 | 1062 | NodeResult.objects.store_data( | 1062 | NodeResult.objects.store_data( |
138 | 1063 | node, filename, script_result, RESULT_TYPE.COMMISSIONING, | 1063 | node, filename, script_result, RESULT_TYPE.COMMISSIONING, |
140 | 1064 | Bin(data)) | 1064 | wrap_bin(data)) |
141 | 1065 | other_node = factory.make_Node(status=NODE_STATUS.NEW) | 1065 | other_node = factory.make_Node(status=NODE_STATUS.NEW) |
142 | 1066 | other_node.start_commissioning(factory.make_admin()) | 1066 | other_node.start_commissioning(factory.make_admin()) |
143 | 1067 | self.assertEqual( | 1067 | self.assertEqual( |
144 | 1068 | 1068 | ||
145 | === modified file 'src/maasserver/testing/factory.py' | |||
146 | --- src/maasserver/testing/factory.py 2014-09-19 12:48:48 +0000 | |||
147 | +++ src/maasserver/testing/factory.py 2014-09-23 04:43:27 +0000 | |||
148 | @@ -76,7 +76,7 @@ | |||
149 | 76 | import maastesting.factory | 76 | import maastesting.factory |
150 | 77 | from maastesting.factory import NO_VALUE | 77 | from maastesting.factory import NO_VALUE |
151 | 78 | from metadataserver.enum import RESULT_TYPE | 78 | from metadataserver.enum import RESULT_TYPE |
153 | 79 | from metadataserver.fields import Bin | 79 | from metadataserver.fields import wrap_bin |
154 | 80 | from metadataserver.models import ( | 80 | from metadataserver.models import ( |
155 | 81 | CommissioningScript, | 81 | CommissioningScript, |
156 | 82 | NodeResult, | 82 | NodeResult, |
157 | @@ -445,7 +445,7 @@ | |||
158 | 445 | script_result = random.randint(0, 10) | 445 | script_result = random.randint(0, 10) |
159 | 446 | ncr = NodeResult( | 446 | ncr = NodeResult( |
160 | 447 | node=node, name=name, script_result=script_result, | 447 | node=node, name=name, script_result=script_result, |
162 | 448 | result_type=RESULT_TYPE.COMMISSIONING, data=Bin(data)) | 448 | result_type=RESULT_TYPE.COMMISSIONING, data=wrap_bin(data)) |
163 | 449 | ncr.save() | 449 | ncr.save() |
164 | 450 | return ncr | 450 | return ncr |
165 | 451 | 451 | ||
166 | @@ -462,7 +462,7 @@ | |||
167 | 462 | script_result = random.randint(0, 10) | 462 | script_result = random.randint(0, 10) |
168 | 463 | ncr = NodeResult( | 463 | ncr = NodeResult( |
169 | 464 | node=node, name=name, script_result=script_result, | 464 | node=node, name=name, script_result=script_result, |
171 | 465 | result_type=RESULT_TYPE.INSTALLING, data=Bin(data)) | 465 | result_type=RESULT_TYPE.INSTALLING, data=wrap_bin(data)) |
172 | 466 | ncr.save() | 466 | ncr.save() |
173 | 467 | return ncr | 467 | return ncr |
174 | 468 | 468 | ||
175 | @@ -707,7 +707,7 @@ | |||
176 | 707 | if content is None: | 707 | if content is None: |
177 | 708 | content = b'content:' + self.make_string().encode('ascii') | 708 | content = b'content:' + self.make_string().encode('ascii') |
178 | 709 | return CommissioningScript.objects.create( | 709 | return CommissioningScript.objects.create( |
180 | 710 | name=name, content=Bin(content)) | 710 | name=name, content=wrap_bin(content)) |
181 | 711 | 711 | ||
182 | 712 | def make_DownloadProgress(self, nodegroup=None, filename=None, | 712 | def make_DownloadProgress(self, nodegroup=None, filename=None, |
183 | 713 | size=NO_VALUE, bytes_downloaded=NO_VALUE, | 713 | size=NO_VALUE, bytes_downloaded=NO_VALUE, |
184 | 714 | 714 | ||
185 | === modified file 'src/maasserver/tests/test_third_party_drivers.py' | |||
186 | --- src/maasserver/tests/test_third_party_drivers.py 2014-09-10 16:20:31 +0000 | |||
187 | +++ src/maasserver/tests/test_third_party_drivers.py 2014-09-23 04:43:27 +0000 | |||
188 | @@ -28,7 +28,7 @@ | |||
189 | 28 | from maastesting import root | 28 | from maastesting import root |
190 | 29 | from maastesting.testcase import MAASTestCase | 29 | from maastesting.testcase import MAASTestCase |
191 | 30 | from metadataserver.enum import RESULT_TYPE | 30 | from metadataserver.enum import RESULT_TYPE |
193 | 31 | from metadataserver.fields import Bin | 31 | from metadataserver.fields import wrap_bin |
194 | 32 | from metadataserver.models import ( | 32 | from metadataserver.models import ( |
195 | 33 | commissioningscript, | 33 | commissioningscript, |
196 | 34 | NodeResult, | 34 | NodeResult, |
197 | @@ -42,7 +42,7 @@ | |||
198 | 42 | node = factory.make_Node() | 42 | node = factory.make_Node() |
199 | 43 | NodeResult.objects.store_data( | 43 | NodeResult.objects.store_data( |
200 | 44 | node, commissioningscript.LIST_MODALIASES_OUTPUT_NAME, | 44 | node, commissioningscript.LIST_MODALIASES_OUTPUT_NAME, |
202 | 45 | 0, RESULT_TYPE.COMMISSIONING, Bin(test_data)) | 45 | 0, RESULT_TYPE.COMMISSIONING, wrap_bin(test_data)) |
203 | 46 | 46 | ||
204 | 47 | aliases = node_modaliases(node) | 47 | aliases = node_modaliases(node) |
205 | 48 | self.assertEqual(['hulla', 'baloo'], aliases) | 48 | self.assertEqual(['hulla', 'baloo'], aliases) |
206 | 49 | 49 | ||
207 | === modified file 'src/metadataserver/api.py' | |||
208 | --- src/metadataserver/api.py 2014-08-22 16:25:29 +0000 | |||
209 | +++ src/metadataserver/api.py 2014-09-23 04:43:27 +0000 | |||
210 | @@ -67,7 +67,7 @@ | |||
211 | 67 | RESULT_TYPE, | 67 | RESULT_TYPE, |
212 | 68 | SIGNAL_STATUS, | 68 | SIGNAL_STATUS, |
213 | 69 | ) | 69 | ) |
215 | 70 | from metadataserver.fields import Bin | 70 | from metadataserver.fields import wrap_bin |
216 | 71 | from metadataserver.models import ( | 71 | from metadataserver.models import ( |
217 | 72 | CommissioningScript, | 72 | CommissioningScript, |
218 | 73 | NodeKey, | 73 | NodeKey, |
219 | @@ -210,7 +210,7 @@ | |||
220 | 210 | raw_content = uploaded_file.read() | 210 | raw_content = uploaded_file.read() |
221 | 211 | NodeResult.objects.store_data( | 211 | NodeResult.objects.store_data( |
222 | 212 | node, name, script_result=0, | 212 | node, name, script_result=0, |
224 | 213 | result_type=RESULT_TYPE.INSTALLING, data=Bin(raw_content)) | 213 | result_type=RESULT_TYPE.INSTALLING, data=wrap_bin(raw_content)) |
225 | 214 | 214 | ||
226 | 215 | def _store_commissioning_results(self, node, request): | 215 | def _store_commissioning_results(self, node, request): |
227 | 216 | """Store commissioning result files for `node`.""" | 216 | """Store commissioning result files for `node`.""" |
228 | @@ -224,7 +224,8 @@ | |||
229 | 224 | exit_status=script_result) | 224 | exit_status=script_result) |
230 | 225 | NodeResult.objects.store_data( | 225 | NodeResult.objects.store_data( |
231 | 226 | node, name, script_result, | 226 | node, name, script_result, |
233 | 227 | result_type=RESULT_TYPE.COMMISSIONING, data=Bin(raw_content)) | 227 | result_type=RESULT_TYPE.COMMISSIONING, |
234 | 228 | data=wrap_bin(raw_content)) | ||
235 | 228 | 229 | ||
236 | 229 | @operation(idempotent=False) | 230 | @operation(idempotent=False) |
237 | 230 | def signal(self, request, version=None, mac=None): | 231 | def signal(self, request, version=None, mac=None): |
238 | 231 | 232 | ||
239 | === modified file 'src/metadataserver/fields.py' | |||
240 | --- src/metadataserver/fields.py 2013-10-21 05:31:09 +0000 | |||
241 | +++ src/metadataserver/fields.py 2014-09-23 04:43:27 +0000 | |||
242 | @@ -1,4 +1,4 @@ | |||
244 | 1 | # Copyright 2012 Canonical Ltd. This software is licensed under the | 1 | # Copyright 2012-2014 Canonical Ltd. This software is licensed under the |
245 | 2 | # GNU Affero General Public License version 3 (see the file LICENSE). | 2 | # GNU Affero General Public License version 3 (see the file LICENSE). |
246 | 3 | 3 | ||
247 | 4 | """Custom field types for the metadata server.""" | 4 | """Custom field types for the metadata server.""" |
248 | @@ -14,6 +14,7 @@ | |||
249 | 14 | __metaclass__ = type | 14 | __metaclass__ = type |
250 | 15 | __all__ = [ | 15 | __all__ = [ |
251 | 16 | 'BinaryField', | 16 | 'BinaryField', |
252 | 17 | 'wrap_bin', | ||
253 | 17 | ] | 18 | ] |
254 | 18 | 19 | ||
255 | 19 | from base64 import ( | 20 | from base64 import ( |
256 | @@ -29,6 +30,26 @@ | |||
257 | 29 | from south.modelsinspector import add_introspection_rules | 30 | from south.modelsinspector import add_introspection_rules |
258 | 30 | 31 | ||
259 | 31 | 32 | ||
260 | 33 | def wrap_bin(binary_data): | ||
261 | 34 | """Wrap a `bytes` into a `Bin`. | ||
262 | 35 | |||
263 | 36 | Use this to construct a `Bin`, a marker class with which we wrap binary | ||
264 | 37 | data. Otherwise Django field conversions won't be able to tell a binary | ||
265 | 38 | value as found in the database from one that's already been "converted" to | ||
266 | 39 | a Python object. | ||
267 | 40 | |||
268 | 41 | :param binary_data: A `bytes` object. Nothing else is accepted. | ||
269 | 42 | :return: A `Bin` containing `binary_data`. | ||
270 | 43 | """ | ||
271 | 44 | # We can't give `Bin` its own constructor to check this, because the | ||
272 | 45 | # upcall to bytes.__init__ triggers a DeprecationWarning about | ||
273 | 46 | # object.__init__ no longer accepting arguments. (This happens even with | ||
274 | 47 | # a direct upcall; it's not just super() that causes it.) | ||
275 | 48 | assert isinstance(binary_data, bytes), ( | ||
276 | 49 | "Not a binary string: '%s'." % repr(binary_data)) | ||
277 | 50 | return Bin(binary_data) | ||
278 | 51 | |||
279 | 52 | |||
280 | 32 | class Bin(bytes): | 53 | class Bin(bytes): |
281 | 33 | """Wrapper class to convince django that a string is really binary. | 54 | """Wrapper class to convince django that a string is really binary. |
282 | 34 | 55 | ||
283 | @@ -39,24 +60,12 @@ | |||
284 | 39 | can stay as it is). The line between bytes and unicode is dangerously | 60 | can stay as it is). The line between bytes and unicode is dangerously |
285 | 40 | thin. | 61 | thin. |
286 | 41 | 62 | ||
288 | 42 | So, to store a value in a BinaryField, wrap it in a Bin: | 63 | So, to store a value in a BinaryField, wrap it in a Bin. Use the |
289 | 64 | `wrap_bin` factory: | ||
290 | 43 | 65 | ||
292 | 44 | my_model_object.binary_data = Bin(b"\x01\x02\x03") | 66 | my_model_object.binary_data = wrap_bin(b"\x01\x02\x03") |
293 | 45 | """ | 67 | """ |
294 | 46 | 68 | ||
295 | 47 | def __init__(self, initializer): | ||
296 | 48 | """Wrap a bytes. | ||
297 | 49 | |||
298 | 50 | :param initializer: Binary string of data for this Bin. This must | ||
299 | 51 | be a bytes. Anything else is almost certainly a mistake, so e.g. | ||
300 | 52 | this constructor will refuse to render None as b'None'. | ||
301 | 53 | :type initializer: bytes | ||
302 | 54 | """ | ||
303 | 55 | if not isinstance(initializer, bytes): | ||
304 | 56 | raise AssertionError( | ||
305 | 57 | "Not a binary string: '%s'" % repr(initializer)) | ||
306 | 58 | super(Bin, self).__init__(initializer) | ||
307 | 59 | |||
308 | 60 | def __emittable__(self): | 69 | def __emittable__(self): |
309 | 61 | """Emit base-64 encoded bytes. | 70 | """Emit base-64 encoded bytes. |
310 | 62 | 71 | ||
311 | 63 | 72 | ||
312 | === modified file 'src/metadataserver/migrations/0012_commission_result_binary_data_recode.py' | |||
313 | --- src/metadataserver/migrations/0012_commission_result_binary_data_recode.py 2014-03-27 04:15:45 +0000 | |||
314 | +++ src/metadataserver/migrations/0012_commission_result_binary_data_recode.py 2014-09-23 04:43:27 +0000 | |||
315 | @@ -1,7 +1,6 @@ | |||
316 | 1 | # -*- coding: utf-8 -*- | 1 | # -*- coding: utf-8 -*- |
317 | 2 | import datetime | 2 | import datetime |
318 | 3 | 3 | ||
319 | 4 | from django.db import models | ||
320 | 5 | from south.db import db | 4 | from south.db import db |
321 | 6 | from south.v2 import DataMigration | 5 | from south.v2 import DataMigration |
322 | 7 | 6 | ||
323 | @@ -10,9 +9,9 @@ | |||
324 | 10 | 9 | ||
325 | 11 | def forwards(self, orm): | 10 | def forwards(self, orm): |
326 | 12 | "Write your forwards methods here." | 11 | "Write your forwards methods here." |
328 | 13 | from metadataserver.fields import Bin | 12 | from metadataserver.fields import wrap_bin |
329 | 14 | for result in orm.NodeCommissionResult.objects.all(): | 13 | for result in orm.NodeCommissionResult.objects.all(): |
331 | 15 | result.data_bin = Bin(result.data.encode("utf-8")) | 14 | result.data_bin = wrap_bin(result.data.encode("utf-8")) |
332 | 16 | result.save() | 15 | result.save() |
333 | 17 | 16 | ||
334 | 18 | def backwards(self, orm): | 17 | def backwards(self, orm): |
335 | 19 | 18 | ||
336 | === modified file 'src/metadataserver/models/commissioningscript.py' | |||
337 | --- src/metadataserver/models/commissioningscript.py 2014-08-13 21:49:35 +0000 | |||
338 | +++ src/metadataserver/models/commissioningscript.py 2014-09-23 04:43:27 +0000 | |||
339 | @@ -1,4 +1,4 @@ | |||
341 | 1 | # Copyright 2012, 2013 Canonical Ltd. This software is licensed under the | 1 | # Copyright 2012-2014 Canonical Ltd. This software is licensed under the |
342 | 2 | # GNU Affero General Public License version 3 (see the file LICENSE). | 2 | # GNU Affero General Public License version 3 (see the file LICENSE). |
343 | 3 | 3 | ||
344 | 4 | """Custom commissioning scripts, and their database backing.""" | 4 | """Custom commissioning scripts, and their database backing.""" |
345 | @@ -47,8 +47,8 @@ | |||
346 | 47 | from metadataserver import DefaultMeta | 47 | from metadataserver import DefaultMeta |
347 | 48 | from metadataserver.enum import RESULT_TYPE | 48 | from metadataserver.enum import RESULT_TYPE |
348 | 49 | from metadataserver.fields import ( | 49 | from metadataserver.fields import ( |
349 | 50 | Bin, | ||
350 | 51 | BinaryField, | 50 | BinaryField, |
351 | 51 | wrap_bin, | ||
352 | 52 | ) | 52 | ) |
353 | 53 | from metadataserver.models.noderesult import NodeResult | 53 | from metadataserver.models.noderesult import NodeResult |
354 | 54 | 54 | ||
355 | @@ -467,7 +467,7 @@ | |||
356 | 467 | assert isinstance(output, bytes) | 467 | assert isinstance(output, bytes) |
357 | 468 | NodeResult.objects.store_data( | 468 | NodeResult.objects.store_data( |
358 | 469 | node, name, script_result=exit_status, | 469 | node, name, script_result=exit_status, |
360 | 470 | result_type=RESULT_TYPE.COMMISSIONING, data=Bin(output)) | 470 | result_type=RESULT_TYPE.COMMISSIONING, data=wrap_bin(output)) |
361 | 471 | if name in BUILTIN_COMMISSIONING_SCRIPTS: | 471 | if name in BUILTIN_COMMISSIONING_SCRIPTS: |
362 | 472 | postprocess_hook = BUILTIN_COMMISSIONING_SCRIPTS[name]['hook'] | 472 | postprocess_hook = BUILTIN_COMMISSIONING_SCRIPTS[name]['hook'] |
363 | 473 | postprocess_hook(node=node, output=output, exit_status=exit_status) | 473 | postprocess_hook(node=node, output=output, exit_status=exit_status) |
364 | 474 | 474 | ||
365 | === modified file 'src/metadataserver/models/nodeuserdata.py' | |||
366 | --- src/metadataserver/models/nodeuserdata.py 2014-09-02 12:34:10 +0000 | |||
367 | +++ src/metadataserver/models/nodeuserdata.py 2014-09-23 04:43:27 +0000 | |||
368 | @@ -1,4 +1,4 @@ | |||
370 | 1 | # Copyright 2012 Canonical Ltd. This software is licensed under the | 1 | # Copyright 2012-2014 Canonical Ltd. This software is licensed under the |
371 | 2 | # GNU Affero General Public License version 3 (see the file LICENSE). | 2 | # GNU Affero General Public License version 3 (see the file LICENSE). |
372 | 3 | 3 | ||
373 | 4 | """Node user-data for cloud-init's use.""" | 4 | """Node user-data for cloud-init's use.""" |
374 | @@ -25,8 +25,8 @@ | |||
375 | 25 | from maasserver.models.cleansave import CleanSave | 25 | from maasserver.models.cleansave import CleanSave |
376 | 26 | from metadataserver import DefaultMeta | 26 | from metadataserver import DefaultMeta |
377 | 27 | from metadataserver.fields import ( | 27 | from metadataserver.fields import ( |
378 | 28 | Bin, | ||
379 | 29 | BinaryField, | 28 | BinaryField, |
380 | 29 | wrap_bin, | ||
381 | 30 | ) | 30 | ) |
382 | 31 | 31 | ||
383 | 32 | 32 | ||
384 | @@ -53,7 +53,7 @@ | |||
385 | 53 | 53 | ||
386 | 54 | def _set(self, node, data): | 54 | def _set(self, node, data): |
387 | 55 | """Set actual user data for a node. Not usable if data is None.""" | 55 | """Set actual user data for a node. Not usable if data is None.""" |
389 | 56 | wrapped_data = Bin(data) | 56 | wrapped_data = wrap_bin(data) |
390 | 57 | (existing_entry, created) = self.get_or_create( | 57 | (existing_entry, created) = self.get_or_create( |
391 | 58 | node=node, defaults={'data': wrapped_data}) | 58 | node=node, defaults={'data': wrapped_data}) |
392 | 59 | if not created: | 59 | if not created: |
393 | @@ -72,7 +72,7 @@ | |||
394 | 72 | self.filter(node__in=nodes).delete() | 72 | self.filter(node__in=nodes).delete() |
395 | 73 | if data is not None: | 73 | if data is not None: |
396 | 74 | self.bulk_create(( | 74 | self.bulk_create(( |
398 | 75 | self.model(node=node, data=Bin(data)) | 75 | self.model(node=node, data=wrap_bin(data)) |
399 | 76 | for node in nodes | 76 | for node in nodes |
400 | 77 | )) | 77 | )) |
401 | 78 | 78 | ||
402 | 79 | 79 | ||
403 | === modified file 'src/metadataserver/models/tests/test_nodecommissionresult.py' | |||
404 | --- src/metadataserver/models/tests/test_nodecommissionresult.py 2014-09-15 14:28:28 +0000 | |||
405 | +++ src/metadataserver/models/tests/test_nodecommissionresult.py 2014-09-23 04:43:27 +0000 | |||
406 | @@ -22,7 +22,7 @@ | |||
407 | 22 | from maasserver.utils.converters import XMLToYAML | 22 | from maasserver.utils.converters import XMLToYAML |
408 | 23 | from maastesting.djangotestcase import DjangoTestCase | 23 | from maastesting.djangotestcase import DjangoTestCase |
409 | 24 | from metadataserver.enum import RESULT_TYPE | 24 | from metadataserver.enum import RESULT_TYPE |
411 | 25 | from metadataserver.fields import Bin | 25 | from metadataserver.fields import wrap_bin |
412 | 26 | from metadataserver.models import NodeResult | 26 | from metadataserver.models import NodeResult |
413 | 27 | from metadataserver.models.commissioningscript import ( | 27 | from metadataserver.models.commissioningscript import ( |
414 | 28 | LLDP_OUTPUT_NAME, | 28 | LLDP_OUTPUT_NAME, |
415 | @@ -133,7 +133,7 @@ | |||
416 | 133 | script_result = randint(0, 10) | 133 | script_result = randint(0, 10) |
417 | 134 | result = NodeResult.objects.store_data( | 134 | result = NodeResult.objects.store_data( |
418 | 135 | node, name=name, script_result=script_result, | 135 | node, name=name, script_result=script_result, |
420 | 136 | result_type=RESULT_TYPE.COMMISSIONING, data=Bin(data)) | 136 | result_type=RESULT_TYPE.COMMISSIONING, data=wrap_bin(data)) |
421 | 137 | result_in_db = NodeResult.objects.get(node=node) | 137 | result_in_db = NodeResult.objects.get(node=node) |
422 | 138 | 138 | ||
423 | 139 | self.assertAttributes(result_in_db, dict(name=name, data=data)) | 139 | self.assertAttributes(result_in_db, dict(name=name, data=data)) |
424 | @@ -148,7 +148,7 @@ | |||
425 | 148 | data = factory.make_bytes(1024 * 1024) | 148 | data = factory.make_bytes(1024 * 1024) |
426 | 149 | NodeResult.objects.store_data( | 149 | NodeResult.objects.store_data( |
427 | 150 | node, name=name, script_result=script_result, | 150 | node, name=name, script_result=script_result, |
429 | 151 | result_type=RESULT_TYPE.COMMISSIONING, data=Bin(data)) | 151 | result_type=RESULT_TYPE.COMMISSIONING, data=wrap_bin(data)) |
430 | 152 | 152 | ||
431 | 153 | self.assertAttributes( | 153 | self.assertAttributes( |
432 | 154 | NodeResult.objects.get(node=node), | 154 | NodeResult.objects.get(node=node), |
433 | 155 | 155 | ||
434 | === modified file 'src/metadataserver/models/tests/test_noderesults.py' | |||
435 | --- src/metadataserver/models/tests/test_noderesults.py 2014-09-10 16:20:31 +0000 | |||
436 | +++ src/metadataserver/models/tests/test_noderesults.py 2014-09-23 04:43:27 +0000 | |||
437 | @@ -45,7 +45,7 @@ | |||
438 | 45 | from maastesting.matchers import MockCalledOnceWith | 45 | from maastesting.matchers import MockCalledOnceWith |
439 | 46 | from maastesting.utils import sample_binary_data | 46 | from maastesting.utils import sample_binary_data |
440 | 47 | from metadataserver.enum import RESULT_TYPE | 47 | from metadataserver.enum import RESULT_TYPE |
442 | 48 | from metadataserver.fields import Bin | 48 | from metadataserver.fields import wrap_bin |
443 | 49 | from metadataserver.models import ( | 49 | from metadataserver.models import ( |
444 | 50 | CommissioningScript, | 50 | CommissioningScript, |
445 | 51 | commissioningscript as cs_module, | 51 | commissioningscript as cs_module, |
446 | @@ -153,7 +153,7 @@ | |||
447 | 153 | def test_scripts_may_be_binary(self): | 153 | def test_scripts_may_be_binary(self): |
448 | 154 | name = make_script_name() | 154 | name = make_script_name() |
449 | 155 | CommissioningScript.objects.create( | 155 | CommissioningScript.objects.create( |
451 | 156 | name=name, content=Bin(sample_binary_data)) | 156 | name=name, content=wrap_bin(sample_binary_data)) |
452 | 157 | stored_script = CommissioningScript.objects.get(name=name) | 157 | stored_script = CommissioningScript.objects.get(name=name) |
453 | 158 | self.assertEqual(sample_binary_data, stored_script.content) | 158 | self.assertEqual(sample_binary_data, stored_script.content) |
454 | 159 | 159 | ||
455 | 160 | 160 | ||
456 | === modified file 'src/metadataserver/tests/test_fields.py' | |||
457 | --- src/metadataserver/tests/test_fields.py 2014-07-18 17:05:57 +0000 | |||
458 | +++ src/metadataserver/tests/test_fields.py 2014-09-23 04:43:27 +0000 | |||
459 | @@ -22,6 +22,7 @@ | |||
460 | 22 | from metadataserver.fields import ( | 22 | from metadataserver.fields import ( |
461 | 23 | Bin, | 23 | Bin, |
462 | 24 | BinaryField, | 24 | BinaryField, |
463 | 25 | wrap_bin, | ||
464 | 25 | ) | 26 | ) |
465 | 26 | from metadataserver.tests.models import BinaryFieldModel | 27 | from metadataserver.tests.models import BinaryFieldModel |
466 | 27 | 28 | ||
467 | @@ -29,24 +30,27 @@ | |||
468 | 29 | class TestBin(MAASServerTestCase): | 30 | class TestBin(MAASServerTestCase): |
469 | 30 | """Test Bin helper class.""" | 31 | """Test Bin helper class.""" |
470 | 31 | 32 | ||
481 | 32 | def test_is_basically_bytes(self): | 33 | def test__is_what_wrap_bin_returns(self): |
482 | 33 | self.assertEqual(b"Hello", Bin(b"Hello")) | 34 | self.assertIsInstance(wrap_bin(b"Hi"), Bin) |
483 | 34 | 35 | ||
484 | 35 | def test_refuses_to_construct_from_unicode(self): | 36 | def test__is_basically_bytes(self): |
485 | 36 | self.assertRaises(AssertionError, Bin, "Hello") | 37 | self.assertEqual(b"Hello", wrap_bin(b"Hello")) |
486 | 37 | 38 | ||
487 | 38 | def test_refuses_to_construct_from_None(self): | 39 | def test__refuses_to_construct_from_unicode(self): |
488 | 39 | self.assertRaises(AssertionError, Bin, None) | 40 | self.assertRaises(AssertionError, wrap_bin, "Hello") |
489 | 40 | 41 | ||
490 | 41 | def test_emits_base64(self): | 42 | def test__refuses_to_construct_from_None(self): |
491 | 43 | self.assertRaises(AssertionError, wrap_bin, None) | ||
492 | 44 | |||
493 | 45 | def test__emits_base64(self): | ||
494 | 42 | # Piston hooks onto an __emittable__() method, if present. | 46 | # Piston hooks onto an __emittable__() method, if present. |
495 | 43 | # Bin() returns a base-64 encoded string so that it can be | 47 | # Bin() returns a base-64 encoded string so that it can be |
496 | 44 | # transmitted in JSON. | 48 | # transmitted in JSON. |
498 | 45 | self.assertEqual(b"", Bin(b"").__emittable__()) | 49 | self.assertEqual(b"", wrap_bin(b"").__emittable__()) |
499 | 46 | example_bytes = factory.make_bytes() | 50 | example_bytes = factory.make_bytes() |
500 | 47 | self.assertEqual( | 51 | self.assertEqual( |
501 | 48 | b64encode(example_bytes), | 52 | b64encode(example_bytes), |
503 | 49 | Bin(example_bytes).__emittable__()) | 53 | wrap_bin(example_bytes).__emittable__()) |
504 | 50 | 54 | ||
505 | 51 | 55 | ||
506 | 52 | class TestBinaryField(TestModelMixin, MAASServerTestCase): | 56 | class TestBinaryField(TestModelMixin, MAASServerTestCase): |
507 | @@ -62,7 +66,7 @@ | |||
508 | 62 | BinaryFieldModel.objects.get(id=binary_item.id).data) | 66 | BinaryFieldModel.objects.get(id=binary_item.id).data) |
509 | 63 | 67 | ||
510 | 64 | def test_stores_and_retrieves_empty_data(self): | 68 | def test_stores_and_retrieves_empty_data(self): |
512 | 65 | binary_item = BinaryFieldModel(data=Bin(b'')) | 69 | binary_item = BinaryFieldModel(data=wrap_bin(b'')) |
513 | 66 | self.assertEqual(b'', binary_item.data) | 70 | self.assertEqual(b'', binary_item.data) |
514 | 67 | binary_item.save() | 71 | binary_item.save() |
515 | 68 | self.assertEqual( | 72 | self.assertEqual( |
516 | @@ -70,7 +74,7 @@ | |||
517 | 70 | 74 | ||
518 | 71 | def test_does_not_truncate_at_zero_bytes(self): | 75 | def test_does_not_truncate_at_zero_bytes(self): |
519 | 72 | data = b"BEFORE THE ZERO\x00AFTER THE ZERO" | 76 | data = b"BEFORE THE ZERO\x00AFTER THE ZERO" |
521 | 73 | binary_item = BinaryFieldModel(data=Bin(data)) | 77 | binary_item = BinaryFieldModel(data=wrap_bin(data)) |
522 | 74 | self.assertEqual(data, binary_item.data) | 78 | self.assertEqual(data, binary_item.data) |
523 | 75 | binary_item.save() | 79 | binary_item.save() |
524 | 76 | self.assertEqual( | 80 | self.assertEqual( |
525 | @@ -78,24 +82,24 @@ | |||
526 | 78 | 82 | ||
527 | 79 | def test_stores_and_retrieves_binary_data(self): | 83 | def test_stores_and_retrieves_binary_data(self): |
528 | 80 | data = b"\x01\x02\xff\xff\xfe\xff\xff\xfe" | 84 | data = b"\x01\x02\xff\xff\xfe\xff\xff\xfe" |
530 | 81 | binary_item = BinaryFieldModel(data=Bin(data)) | 85 | binary_item = BinaryFieldModel(data=wrap_bin(data)) |
531 | 82 | self.assertEqual(data, binary_item.data) | 86 | self.assertEqual(data, binary_item.data) |
532 | 83 | binary_item.save() | 87 | binary_item.save() |
533 | 84 | self.assertEqual( | 88 | self.assertEqual( |
534 | 85 | data, BinaryFieldModel.objects.get(id=binary_item.id).data) | 89 | data, BinaryFieldModel.objects.get(id=binary_item.id).data) |
535 | 86 | 90 | ||
536 | 87 | def test_returns_bytes_not_text(self): | 91 | def test_returns_bytes_not_text(self): |
538 | 88 | binary_item = BinaryFieldModel(data=Bin(b"Data")) | 92 | binary_item = BinaryFieldModel(data=wrap_bin(b"Data")) |
539 | 89 | binary_item.save() | 93 | binary_item.save() |
540 | 90 | retrieved_data = BinaryFieldModel.objects.get(id=binary_item.id).data | 94 | retrieved_data = BinaryFieldModel.objects.get(id=binary_item.id).data |
541 | 91 | self.assertIsInstance(retrieved_data, bytes) | 95 | self.assertIsInstance(retrieved_data, bytes) |
542 | 92 | 96 | ||
543 | 93 | def test_looks_up_data(self): | 97 | def test_looks_up_data(self): |
544 | 94 | data = b"Binary item" | 98 | data = b"Binary item" |
546 | 95 | binary_item = BinaryFieldModel(data=Bin(data)) | 99 | binary_item = BinaryFieldModel(data=wrap_bin(data)) |
547 | 96 | binary_item.save() | 100 | binary_item.save() |
548 | 97 | self.assertEqual( | 101 | self.assertEqual( |
550 | 98 | binary_item, BinaryFieldModel.objects.get(data=Bin(data))) | 102 | binary_item, BinaryFieldModel.objects.get(data=wrap_bin(data))) |
551 | 99 | 103 | ||
552 | 100 | def test_get_default_returns_None(self): | 104 | def test_get_default_returns_None(self): |
553 | 101 | field = BinaryField(null=True) | 105 | field = BinaryField(null=True) |
554 | @@ -104,10 +108,10 @@ | |||
555 | 104 | 108 | ||
556 | 105 | def test_get_default_returns_Bin(self): | 109 | def test_get_default_returns_Bin(self): |
557 | 106 | field = BinaryField(null=True) | 110 | field = BinaryField(null=True) |
560 | 107 | self.patch(field, "default", Bin(b"wotcha")) | 111 | self.patch(field, "default", wrap_bin(b"wotcha")) |
561 | 108 | self.assertEqual(Bin(b"wotcha"), field.get_default()) | 112 | self.assertEqual(wrap_bin(b"wotcha"), field.get_default()) |
562 | 109 | 113 | ||
563 | 110 | def test_get_default_returns_Bin_from_bytes(self): | 114 | def test_get_default_returns_Bin_from_bytes(self): |
564 | 111 | field = BinaryField(null=True) | 115 | field = BinaryField(null=True) |
565 | 112 | self.patch(field, "default", b"wotcha") | 116 | self.patch(field, "default", b"wotcha") |
567 | 113 | self.assertEqual(Bin(b"wotcha"), field.get_default()) | 117 | self.assertEqual(wrap_bin(b"wotcha"), field.get_default()) |
I think there's a much simpler fix, but have a look and check that it does what you need.