x
/
solar
Code Issues Proposed changes

Rework staging to avoid unnecessary children log items creation 

Procedure in staged_log, that was adding log items based on connections,
was reworked to avoid creation of LogItem instances before filtering
those children that weren't changed (with empty diff or connections_diff)

That problem was leading to unpredictable behaviour during updates,
removal and relevant discard/revert scenarios.

Change-Id: I65adb8262fdbe10299c02c54db9d19fb255bceea
This commit is contained in:
Dmitry Shulyak 2016-04-18 17:40:49 +03:00
parent 5ef0be35ca
commit a2f9e37fe8
4 changed files with 195 additions and 131 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
solar/cli/system_log.py
View File

@ -46,7 +46,7 @@ def validate():
def stage(action, name, tag, d):
if action and (name or tag):
resource.stage_resources(name or tag, action)
log = change.staged_log(populate_with_changes=True)
log = change.staged_log()
for item in log:
click.echo(data.compact(item))
if d:

22
solar/core/resource/resource.py
View File

@ -75,7 +75,6 @@ class Resource(object):
tags.append('resource={}'.format(name))
inputs = metadata.get('input', {})
self.auto_extend_inputs(inputs)
self.db_obj = DBResource.from_dict(
name,
@ -212,19 +211,22 @@ class Resource(object):
self.db_obj.save_lazy()
return
def update(self, args):
for k, v in args.items():
self.db_obj.inputs[k] = v
self.db_obj.save_lazy()
def _guess_log_item(self, resource_obj):
# created state will be changed during commit
if self.db_obj.state != RESOURCE_STATE.created.name:
if resource_obj.db_obj.state != RESOURCE_STATE.created.name:
action = 'update'
else:
action = 'run'
LogItem.new(
{'resource': self.name,
{'resource': resource_obj.name,
'action': action,
'tags': self.tags}).save_lazy()
'tags': resource_obj.tags}).save_lazy()
def update(self, args):
for k, v in args.items():
self.db_obj.inputs[k] = v
self.db_obj.save_lazy()
self._guess_log_item(self)
def delete(self):
return self.db_obj.delete()
@ -333,9 +335,7 @@ class Resource(object):
use_defaults=False):
mapping = get_mapping(self, receiver, mapping)
self._connect_inputs(receiver, mapping)
LogItem.new({'resource': receiver.name,
'action': 'run',
'tags': receiver.tags}).save_lazy()
self._guess_log_item(receiver)
# signals.connect(self, receiver, mapping=mapping)
# TODO: implement events
if use_defaults:

146
solar/system_log/change.py
View File

@ -12,6 +12,8 @@
# License for the specific language governing permissions and limitations
# under the License.
from collections import namedtuple
import dictdiffer
import networkx as nx
@ -53,59 +55,78 @@ def create_diff(staged, commited):
return listify(res)
def populate_log_item(log_item):
resource_obj = resource.load(log_item.resource)
commited = resource_obj.load_commited()
log_item.base_path = resource_obj.base_path
if resource_obj.to_be_removed():
resource_args = {}
resource_connections = []
else:
resource_args = resource_obj.args
resource_connections = resource_obj.connections
class Diff(namedtuple('Diff', ['resource', 'diff', 'connections', 'path'])):
if commited.state == RESOURCE_STATE.removed.name:
commited_args = {}
commited_connections = []
else:
commited_args = commited.inputs
commited_connections = commited.connections
def to_log_item(self, action):
return LogItem.new(
{'resource': self.resource,
'diff': self.diff,
'connections_diff': self.connections,
'base_path': self.path,
'action': action,
'log': 'staged'})
log_item.diff = create_diff(resource_args, commited_args)
log_item.connections_diff = create_sorted_diff(
resource_connections, commited_connections)
def to_update(self):
return self.to_log_item('update')
def to_run(self):
return self.to_log_item('run')
def to_remove(self):
return self.to_log_item('remove')
@classmethod
def create_from_resource(cls, resource_obj):
commited = resource_obj.load_commited()
if resource_obj.to_be_removed():
resource_args = {}
resource_connections = []
else:
resource_args = resource_obj.args
resource_connections = resource_obj.connections
if commited.state == RESOURCE_STATE.removed.name:
commited_args = {}
commited_connections = []
else:
commited_args = commited.inputs
commited_connections = commited.connections
return cls(resource_obj.name,
create_diff(resource_args, commited_args),
create_sorted_diff(
resource_connections, commited_connections),
resource_obj.base_path)
def create_run(resource_obj):
return Diff.create_from_resource(resource_obj).to_run()
def create_remove(resource_obj):
return Diff.create_from_resource(resource_obj).to_remove()
def create_update(resource_obj):
return Diff.create_from_resource(resource_obj).to_update()
def populate_log_item(log_item, diff=None):
if diff is None:
diff = Diff.create_from_resource(resource.load(log_item.resource))
log_item.diff = diff.diff
log_item.connections_diff = diff.connections
log_item.base_path = diff.path
return log_item
def create_logitem(resource, action, populate=True):
"""Create log item in staged log
:param resource: basestring
:param action: basestring
"""
log_item = LogItem.new(
{'resource': resource,
'action': action,
'log': 'staged'})
if populate:
populate_log_item(log_item)
return log_item
def create_run(resource):
return create_logitem(resource, 'run')
def create_remove(resource):
return create_logitem(resource, 'remove')
def create_sorted_diff(staged, commited):
staged.sort()
commited.sort()
return create_diff(staged, commited)
def staged_log(populate_with_changes=True):
def staged_log():
"""Staging procedure takes manually created log items, populate them
with diff and connections diff
@ -127,19 +148,20 @@ def staged_log(populate_with_changes=True):
resources_names.add(log_item.resource)
log_actions.add(log_item.log_action)
without_duplicates.append(log_item)
utils.solar_map(lambda li: populate_log_item(li),
without_duplicates, concurrency=10)
# this is backward compatible change, there might better way
# to "guess" child actions
childs = filter(lambda child: child.name not in resources_names,
resource.load_childs(list(resources_names)))
child_log_items = filter(
lambda li: li.diff or li.connections_diff,
utils.solar_map(create_run, [c.name for c in childs], concurrency=10))
for log_item in child_log_items + without_duplicates:
diffs = filter(
lambda d: d.diff or d.connections,
utils.solar_map(Diff.create_from_resource, childs, concurrency=10))
update_items = utils.solar_map(
lambda d: d.to_update(), diffs, concurrency=10)
for log_item in update_items + without_duplicates:
log_item.save_lazy()
return without_duplicates + child_log_items
return without_duplicates + update_items
def send_to_orchestration(tags=None):
@ -184,11 +206,11 @@ def _get_args_to_update(args, connections):
def is_create(logitem):
return all((item[0] == 'add' for item in logitem.diff))
return logitem.action == 'run'
def is_update(logitem):
return any((item[0] == 'change' for item in logitem.diff))
return logitem.action == 'update'
def revert_uids(uids):
@ -301,19 +323,23 @@ def _discard_run(item):
resource.load(item.resource).remove(force=True)
def discard_single(item):
if is_update(item):
_discard_update(item)
elif item.action == CHANGES.remove.name:
_discard_remove(item)
elif is_create(item):
_discard_run(item)
else:
log.debug('Action %s for resource %s is a side'
' effect of another action', item.action, item.res)
item.delete()
def discard_uids(uids):
items = filter(bool, LogItem.multi_get(uids))
for item in items:
if is_update(item):
_discard_update(item)
elif item.action == CHANGES.remove.name:
_discard_remove(item)
elif is_create(item):
_discard_run(item)
else:
log.debug('Action %s for resource %s is a side'
' effect of another action', item.action, item.res)
item.delete()
discard_single(item)
def discard_uid(uid):

156
solar/test/test_system_log_api.py
View File

@ -24,54 +24,57 @@ from solar.system_log import change
from solar.system_log import operations
def create_resource(name, tags=None):
def create_resource(name, tags=None, inputs=None):
if inputs is None:
inputs = {'a': ''}
meta_inputs = {}
for key, value in inputs.items():
if not isinstance(value, basestring):
raise Exception('Only strings are allowed')
meta_inputs[key] = {'value': value, 'schema': 'str'}
res = DBResource.from_dict(
name,
{'name': name,
'base_path': 'x',
'state': resource.RESOURCE_STATE.created.name,
'tags': tags or [],
'meta_inputs': {'a': {'value': None,
'schema': 'str'}}})
'meta_inputs': meta_inputs,
'inputs': meta_inputs})
res.save_lazy()
return res
return resource.Resource(res)
def test_revert_update():
prev = {'a': '9'}
new = {'a': '10'}
res = create_resource('test1')
res.save()
action = 'run'
resource_obj = resource.load(res.name)
ModelMeta.save_all_lazy()
resource_obj.update(prev)
logitem = change.create_logitem(res.name, action)
res.update(prev)
ModelMeta.save_all_lazy()
logitem = change.staged_log()[0]
operations.commit_log_item(logitem)
resource_obj.update(new)
logitem = change.create_logitem(res.name, action)
res.update(new)
ModelMeta.save_all_lazy()
logitem = change.staged_log()[0]
uid = logitem.uid
assert logitem.diff == [['change', 'a', ['9', '10']]]
operations.commit_log_item(logitem)
assert resource_obj.args == new
assert res.args == new
change.revert(uid)
assert resource_obj.args == {'a': '9'}
assert res.args == {'a': '9'}
def test_revert_update_connected():
res1 = create_resource('test1')
res1.inputs['a'] = '9'
res1.save_lazy()
res1.update({'a': '9'})
res2 = create_resource('test2')
res2.inputs['a'] = ''
res2.save_lazy()
res2.update({'a': ''})
res3 = create_resource('test3')
res3.inputs['a'] = ''
res3.save_lazy()
res3.update({'a': ''})
res1 = resource.load('test1')
res2 = resource.load('test2')
@ -80,17 +83,15 @@ def test_revert_update_connected():
res2.connect(res3)
ModelMeta.save_all_lazy()
staged_log = map(lambda res: change.create_run(res.name),
(res1, res2, res3))
assert len(staged_log) == 3
staged_items = change.staged_log()
assert len(staged_items) == 3
for item in staged_log:
for item in staged_items:
assert item.action == 'run'
operations.commit_log_item(item)
res1.disconnect(res2)
staged_log = map(lambda res: change.create_run(res.name),
(res2, res3))
staged_log = change.staged_log()
to_revert = []
for item in staged_log:
@ -101,18 +102,54 @@ def test_revert_update_connected():
change.revert_uids(sorted(to_revert, reverse=True))
ModelMeta.save_all_lazy()
staged_log = map(lambda res: change.create_run(res.name),
(res2, res3))
staged_log = change.staged_log()
for item in staged_log:
assert item.diff == [['change', 'a', ['', '9']]]
def test_only_relevant_child_updated():
res1, res2, res3 = (
create_resource(
name, inputs={'a': '', 'b': ''}) for name in ('t1', 't2', 't3'))
res1.update({'a': '9', 'b': '10'})
res1.connect(res2, {'a': 'a'})
res1.connect(res3, {'b': 'b'})
ModelMeta.save_all_lazy()
# currently childs added as a side effect of staged log, thus we need
# to run it before commiting changes
assert set(s.resource for s in change.staged_log()) == {'t1', 't2', 't3'}
change.commit_all()
res1.update({'a': '12'})
ModelMeta.save_all_lazy()
# t3 not updated because "a" connected only to t2
assert set(s.resource for s in change.staged_log()) == {'t1', 't2'}
def test_discard_removed_with_childs_not_affected():
res1, res2, res3 = (
create_resource(
name, inputs={'a': '', 'b': ''}) for name in ('t1', 't2', 't3'))
res1.update({'a': '9', 'b': '10'})
res1.connect(res2, {'a': 'a'})
res1.connect(res3, {'b': 'b'})
ModelMeta.save_all_lazy()
change.staged_log()
change.commit_all()
res1.remove()
ModelMeta.save_all_lazy()
staged_items = change.staged_log()
assert len(staged_items) == 1
assert staged_items[0].log_action == 't1.remove'
change.discard_all()
assert not res1.to_be_removed()
assert not res2.to_be_removed()
assert not res3.to_be_removed()
assert change.staged_log() == []
def test_revert_removal():
res = create_resource('test1')
res.inputs['a'] = '9'
res.save_lazy()
commited = CommitedResource.from_dict('test1', {'inputs': {'a': '9'},
'state': 'operational'})
commited.save_lazy()
@ -121,8 +158,9 @@ def test_revert_removal():
resource_obj.remove()
ModelMeta.save_all_lazy()
log_item = change.create_remove(resource_obj.name)
log_item.save()
staged_items = change.staged_log()
assert len(staged_items) == 1
log_item = staged_items[0]
uid = log_item.uid
assert log_item.diff == [['remove', '', [['a', '9']]]]
operations.commit_log_item(log_item)
@ -150,10 +188,8 @@ def test_revert_removal():
def test_revert_create():
res = create_resource('test1')
res.inputs['a'] = '9'
res.save_lazy()
logitem = change.create_run(res.name)
res.db_obj.inputs['a'] = '9'
logitem = change.create_run(res)
assert logitem.diff == [['add', '', [['a', '9']]]]
uid = logitem.uid
operations.commit_log_item(logitem)
@ -173,13 +209,13 @@ def test_revert_create():
def test_discard_all_pending_changes_resources_created():
res1 = create_resource('test1')
res1.inputs['a'] = '9'
res1.save_lazy()
res1.db_obj.inputs['a'] = '9'
res1.db_obj.save_lazy()
res2 = create_resource('test2')
res2.inputs['a'] = '0'
res2.save_lazy()
staged_log = map(change.create_run, (res1.name, res2.name))
res2.db_obj.inputs['a'] = '0'
res2.db_obj.save_lazy()
staged_log = map(change.create_run, (res1, res2))
change.discard_all()
staged_log = change.staged_log()
@ -189,14 +225,14 @@ def test_discard_all_pending_changes_resources_created():
def test_discard_connection():
res1 = create_resource('test1')
res1.inputs['a'] = '9'
res1.save_lazy()
res1.db_obj.inputs['a'] = '9'
res1.db_obj.save_lazy()
res2 = create_resource('test2')
res2.inputs['a'] = '0'
res2.save_lazy()
res2.db_obj.inputs['a'] = '0'
res2.db_obj.save_lazy()
staged_log = map(change.create_run, (res1.name, res2.name))
staged_log = map(change.create_run, (res1, res2))
for item in staged_log:
operations.commit_log_item(item)
@ -214,8 +250,8 @@ def test_discard_connection():
def test_discard_removed():
res1 = create_resource('test1')
res1.inputs['a'] = '9'
res1.save_lazy()
res1.db_obj.inputs['a'] = '9'
res1.db_obj.save_lazy()
res1 = resource.load('test1')
res1.remove()
@ -231,16 +267,14 @@ def test_discard_removed():
def test_discard_update():
res1 = create_resource('test1')
res1.inputs['a'] = '9'
res1.save_lazy()
operations.commit_log_item(change.create_run(res1.name))
res1 = resource.load('test1')
res1.db_obj.inputs['a'] = '9'
operations.commit_log_item(change.create_run(res1))
res1.update({'a': '11'})
ModelMeta.save_all_lazy()
assert len(change.staged_log()) == 1
assert res1.args == {'a': '11'}
change.discard_all()
change.discard_single(change.staged_log()[0])
assert res1.args == {'a': '9'}
@ -270,16 +304,20 @@ def test_stage_and_process_partially():
def test_childs_added_on_stage():
res_0, res_1 = [create_resource(str(n)) for n in range(2)]
res_0.connect(res_1, {'a': 'a'})
ModelMeta.save_all_lazy()
created_log_items = stage_resources(res_0.name, 'run')
assert len(created_log_items) == 1
assert created_log_items[0].resource == res_0.name
for res in (res_0, res_1):
change.create_run(res)
res_0.connect(res_1, {'a': 'a'})
change.staged_log()
ModelMeta.save_all_lazy()
change.commit_all()
res_0.update({'a': '10'})
ModelMeta.save_all_lazy()
staged_log = change.staged_log()
assert len(staged_log) == 2
child_log_item = next(li for li in staged_log
if li.resource == res_1.name)
assert child_log_item.action == 'run'
assert child_log_item.action == 'update'
def test_update_action_after_commit():