zuul/zuul
Code Issues Proposed changes

Merge "Remove config object freezing"

Browse Source
This commit is contained in:
Zuul
2025-01-16 19:22:48 +00:00
committed by Gerrit Code Review
parent 1b8346a8ca 95d0dda15a
commit 5162bd1a31
3 changed files with 8 additions and 192 deletions
Download Patch File Download Diff File Expand all files Collapse all files

98
tests/unit/test_model.py
View File

@@ -15,11 +15,8 @@
import configparser
import collections
import os
import random
import textwrap
import types
import uuid
from unittest import mock
@@ -1219,101 +1216,6 @@ class TestTenant(BaseTestCase):
tenant._addProject(source1_project1_tpc)
class TestFreezable(BaseTestCase):
def test_freezable_object(self):
o = model.Freezable()
o.foo = 1
o.list = []
o.dict = {}
o.odict = collections.OrderedDict()
o.odict2 = collections.OrderedDict()
o1 = model.Freezable()
o1.foo = 1
l1 = [1]
d1 = {'foo': 1}
od1 = {'foo': 1}
o.list.append(o1)
o.list.append(l1)
o.list.append(d1)
o.list.append(od1)
o2 = model.Freezable()
o2.foo = 1
l2 = [1]
d2 = {'foo': 1}
od2 = {'foo': 1}
o.dict['o'] = o2
o.dict['l'] = l2
o.dict['d'] = d2
o.dict['od'] = od2
o3 = model.Freezable()
o3.foo = 1
l3 = [1]
d3 = {'foo': 1}
od3 = {'foo': 1}
o.odict['o'] = o3
o.odict['l'] = l3
o.odict['d'] = d3
o.odict['od'] = od3
seq = list(range(1000))
random.shuffle(seq)
for x in seq:
o.odict2[x] = x
o.freeze()
with testtools.ExpectedException(Exception, "Unable to modify frozen"):
o.bar = 2
with testtools.ExpectedException(AttributeError, "'tuple' object"):
o.list.append(2)
with testtools.ExpectedException(TypeError, "'mappingproxy' object"):
o.dict['bar'] = 2
with testtools.ExpectedException(TypeError, "'mappingproxy' object"):
o.odict['bar'] = 2
with testtools.ExpectedException(Exception, "Unable to modify frozen"):
o1.bar = 2
with testtools.ExpectedException(Exception, "Unable to modify frozen"):
o.list[0].bar = 2
with testtools.ExpectedException(AttributeError, "'tuple' object"):
o.list[1].append(2)
with testtools.ExpectedException(TypeError, "'mappingproxy' object"):
o.list[2]['bar'] = 2
with testtools.ExpectedException(TypeError, "'mappingproxy' object"):
o.list[3]['bar'] = 2
with testtools.ExpectedException(Exception, "Unable to modify frozen"):
o2.bar = 2
with testtools.ExpectedException(Exception, "Unable to modify frozen"):
o.dict['o'].bar = 2
with testtools.ExpectedException(AttributeError, "'tuple' object"):
o.dict['l'].append(2)
with testtools.ExpectedException(TypeError, "'mappingproxy' object"):
o.dict['d']['bar'] = 2
with testtools.ExpectedException(TypeError, "'mappingproxy' object"):
o.dict['od']['bar'] = 2
with testtools.ExpectedException(Exception, "Unable to modify frozen"):
o3.bar = 2
with testtools.ExpectedException(Exception, "Unable to modify frozen"):
o.odict['o'].bar = 2
with testtools.ExpectedException(AttributeError, "'tuple' object"):
o.odict['l'].append(2)
with testtools.ExpectedException(TypeError, "'mappingproxy' object"):
o.odict['d']['bar'] = 2
with testtools.ExpectedException(TypeError, "'mappingproxy' object"):
o.odict['od']['bar'] = 2
# Make sure that mapping proxy applied to an ordered dict
# still shows the ordered behavior.
self.assertTrue(isinstance(o.odict2, types.MappingProxyType))
self.assertEqual(list(o.odict2.keys()), seq)
class TestRef(BaseTestCase):
def test_ref_equality(self):
change1 = model.Change('project1')

22
zuul/configloader.py
View File

@@ -430,7 +430,6 @@ class ImageParser(object):
conf.get('description'))
image.source_context = conf.get('_source_context')
image.start_mark = conf.get('_start_mark')
image.freeze()
return image
@@ -454,7 +453,6 @@ class FlavorParser(object):
flavor = model.Flavor(conf['name'], conf.get('description'))
flavor.source_context = conf.get('_source_context')
flavor.start_mark = conf.get('_start_mark')
flavor.freeze()
return flavor
@@ -484,7 +482,6 @@ class LabelParser(object):
conf.get('max-ready-age'))
label.source_context = conf.get('_source_context')
label.start_mark = conf.get('_start_mark')
label.freeze()
return label
@@ -524,7 +521,6 @@ class SectionParser(object):
section.source_context = conf.get('_source_context')
section.start_mark = conf.get('_start_mark')
section.config = conf
section.freeze()
return section
@@ -561,7 +557,6 @@ class ProviderParser(object):
provider_config.source_context = conf.get('_source_context')
provider_config.start_mark = conf.get('_start_mark')
provider_config.config = conf
provider_config.freeze()
return provider_config
@@ -673,7 +668,6 @@ class NodeSetParser(object):
as_list(conf_group['nodes']))
ns.addGroup(group)
group_names.add(conf_group['name'])
ns.freeze()
return ns
@@ -699,7 +693,6 @@ class SecretParser(object):
s.source_context = conf['_source_context']
s.start_mark = conf['_start_mark']
s.secret_data = conf['data']
s.freeze()
return s
@@ -1233,7 +1226,6 @@ class JobParser(object):
re2.compile(x)
job.failure_output = tuple(sorted(failure_output))
job.freeze()
return job
def _makeZuulRole(self, job, role):
@@ -1287,7 +1279,7 @@ class ProjectTemplateParser(object):
return vs.Schema(project)
def fromYaml(self, conf, validate=True, freeze=True):
def fromYaml(self, conf, validate=True):
conf = copy_safe_config(conf)
if validate:
self.schema(conf)
@@ -1324,8 +1316,6 @@ class ProjectTemplateParser(object):
"or 'unsafe_vars' are not allowed.")
project_template.variables = variables
if freeze:
project_template.freeze()
return project_template
def parseJobList(self, conf, source_context, start_mark, job_list):
@@ -1402,7 +1392,7 @@ class ProjectParser(object):
# Parse the project as a template since they're mostly the
# same.
project_config = self.pcontext.project_template_parser. \
fromYaml(conf, validate=False, freeze=False)
fromYaml(conf, validate=False)
project_config.name = project_name
else:
@@ -1422,7 +1412,7 @@ class ProjectParser(object):
# Parse the project as a template since they're mostly the
# same.
project_config = self.pcontext.project_template_parser.\
fromYaml(conf, validate=False, freeze=False)
fromYaml(conf, validate=False)
project_config.name = project.canonical_name
@@ -1477,7 +1467,6 @@ class ProjectParser(object):
"or 'unsafe_vars' are not allowed.")
project_config.variables = variables
project_config.freeze()
return project_config
@@ -1732,7 +1721,6 @@ class SemaphoreParser(object):
semaphore = model.Semaphore(conf['name'], conf.get('max', 1))
semaphore.source_context = conf.get('_source_context')
semaphore.start_mark = conf.get('_start_mark')
semaphore.freeze()
return semaphore
@@ -1767,7 +1755,6 @@ class QueueParser:
"enabled in order to use dependencies-by-topic")
queue.source_context = conf.get('_source_context')
queue.start_mark = conf.get('_start_mark')
queue.freeze()
return queue
@@ -1821,7 +1808,6 @@ class GlobalSemaphoreParser(object):
self.schema(conf)
semaphore = model.Semaphore(conf['name'], conf.get('max', 1),
global_scope=True)
semaphore.freeze()
return semaphore
@@ -1842,7 +1828,6 @@ class ApiRootParser(object):
self.schema(conf)
api_root = model.ApiRoot(conf.get('authentication-realm'))
api_root.access_rules = conf.get('access-rules', [])
api_root.freeze()
return api_root
@@ -3052,7 +3037,6 @@ class TenantParser(object):
conf = config_project.copy()
name = project.canonical_name
conf.name = name
conf.freeze()
parsed_config.projects.append(conf)
for project in parsed_config.projects:

80
zuul/model.py
View File

@@ -467,72 +467,7 @@ class Attributes(object):
return self.__dict__
class Freezable(object):
"""A mix-in class so that an object can be made immutable"""
def __init__(self):
super(Freezable, self).__setattr__('_frozen', False)
def freeze(self):
"""Make this object immutable"""
def _freezelist(l):
for i, v in enumerate(l):
if isinstance(v, Freezable):
if not v._frozen:
v.freeze()
elif isinstance(v, dict):
l[i] = _freezedict(v)
elif isinstance(v, list):
l[i] = _freezelist(v)
return tuple(l)
def _freezedict(d):
for k, v in list(d.items()):
if isinstance(v, Freezable):
if not v._frozen:
v.freeze()
elif isinstance(v, dict):
d[k] = _freezedict(v)
elif isinstance(v, list):
d[k] = _freezelist(v)
return types.MappingProxyType(d)
_freezedict(self.__dict__)
# Ignore return value from freezedict because __dict__ can't
# be a mappingproxy.
self._frozen = True
@staticmethod
def thaw(data):
"""Thaw the supplied dictionary"""
def _thawlist(l):
l = list(l)
for i, v in enumerate(l):
if isinstance(v, (types.MappingProxyType, dict)):
l[i] = _thawdict(v)
elif isinstance(v, (tuple, list)):
l[i] = _thawlist(v)
return l
def _thawdict(d):
d = dict(d)
for k, v in list(d.items()):
if isinstance(v, (types.MappingProxyType, dict)):
d[k] = _thawdict(v)
elif isinstance(v, (tuple, list)):
d[k] = _thawlist(v)
return d
return _thawdict(data)
def __setattr__(self, name, value):
if self._frozen:
raise Exception("Unable to modify frozen object %s" %
(repr(self),))
super(Freezable, self).__setattr__(name, value)
class ConfigObject(Freezable):
class ConfigObject:
def __init__(self):
super().__init__()
self.source_context = None
@@ -1845,7 +1780,7 @@ class ProviderConfig(ConfigObject):
return ret
def flattenConfig(self, layout):
config = copy.deepcopy(Freezable.thaw(self.config))
config = copy.deepcopy(self.config)
parent_name = self.section
previous_section = None
while parent_name:
@@ -1857,8 +1792,7 @@ class ProviderConfig(ConfigObject):
raise Exception(
f'The section "{previous_section.name}" references a '
'section in a different project.')
parent_config = copy.deepcopy(Freezable.thaw(
parent_section.config))
parent_config = copy.deepcopy(parent_section.config)
config = ProviderConfig._mergeDict(parent_config, config)
parent_name = parent_section.parent
previous_section = parent_section
@@ -3789,8 +3723,6 @@ class Job(ConfigObject):
# If this is a config object, it's frozen, so it's
# safe to shallow copy.
v = getattr(self, k)
if isinstance(v, (dict, types.MappingProxyType)):
v = Freezable.thaw(v)
# On a frozen job, parent=None means a base job
if v is self.BASE_JOB_MARKER:
v = None
@@ -3838,7 +3770,7 @@ class Job(ConfigObject):
# Make a hash of the job configuration for determining whether
# it has been updated.
hasher = hashlib.sha256()
job_dict = Freezable.thaw(self.toDict(tenant))
job_dict = self.toDict(tenant)
# Ignore changes to file matchers since they don't affect
# the content of the job.
for attr in ['files', 'irrelevant_files',
@@ -9056,9 +8988,7 @@ class Layout(object):
# Return an implied semaphore with max=1
# TODO: consider deprecating implied semaphores to avoid typo
# config errors
semaphore = Semaphore(semaphore_name)
semaphore.freeze()
return semaphore
return Semaphore(semaphore_name)
def addQueue(self, queue):
self._addIdenticalObject('Queue', self.queues, queue)