Tree object runner with circular dependency detector #166

Closed
wants to merge 3 commits into
from

3 participants

@arkaitzj

Changed the way objects are executed, instead of preparing(resolve&manifest) everything before running the first object, we walk the dependency tree preparing and running as soon as possible and always preparing after dependencies have been run, following a Depth-First tree traversal.
Dependencies change during execution as we are not aware of autorequires till we have executed the manifest, but this has been handled as well.
Also, circular dependency detection is performed on both standard and auto requirements.
Unit tests fixed where needed and removed where did not make sense, like standalone prepare dependency checking.

@telmich telmich commented on the diff Apr 3, 2013
cdist/config_install.py
- new_objects_created = True
+ deps = list(core.CdistObject.list_objects(self.context.local.object_path,self.context.local.type_path))
+ ready_objects = {}
+ seen_objs = set()
+ while deps:
+ obj = deps[-1]
+ if obj.name in ready_objects:
+ deps.pop()
+ continue
+ seen_objs.add(obj.name)
+ unmet_reqs = [ req for req in obj.requirements if req not in ready_objects ]
+ if unmet_reqs:
+ seen_reqs = [req for req in unmet_reqs if req in seen_objs]
+ if seen_reqs:
+ raise CircularReferenceError(obj,obj.object_from_name(seen_reqs[0]))
+ deps.extend(obj.find_requirements_by_name(unmet_reqs))
@telmich
telmich added a line comment Apr 3, 2013

Running obj.find_requirements_by_name(unmet_reqs) may actually create a list with missing objects: The objects that are going to be created later, but match a wildcard: require="__file/*" __foo will only require the current __file objects, but should actually also require all future created __file objects.

Please prove me wrong.

@telmich
telmich added a line comment Apr 3, 2013

Actually it seems that this behaviour has not been documented - starting internal discussion about how to proceed.

@arkaitzj
arkaitzj added a line comment Apr 3, 2013

Well, I was not aware that wildcards could be used for require= parameters.
The way this is laid out I would rather think its not a good idea, this adds quite some limitations and yet is not effective as a resource collector.
I would rather be in favor of adding collector behavior so that we could somehow "collect and iterate" over all resources that match a pattern, and then do something, this would obviously happen on a post-run, since you can still create resources on manifests based on explorers and explorers could be finding stuff created by other types gencode-remotes.
My 2 cents.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@asteven

@arkaitzj agreed. pattern pased requirements creates more problems then it solves.
@telmich let's ditch it

@telmich

Hey Arkaitz,

I'd love to merge your change, but as we have a lot of dependencies in the dependency resolver (sigh!) this is a dangerous area.

Thus to be able to merge your pull request, I am now creating a test case that tests the correct run time and dependency order.

I am currently establishing it in the dependency_tests branch.

I'd be very thankful if you could also add potential test cases you see.

The idea is to have this test case independent of the resolver/object code (as far as possible) to test the correct workflow from outside (maybe using mock types or similar approaches).

@telmich

I've just given your pull request a try, there must be at least one bug:

INFO: recomy-staging.panter.ch: Generating and executing code for __directory/home/rails/app/releases
INFO: recomy-staging.panter.ch: Generating and executing code for __panter_rails_railshome/singleton

at this point cdist uses 100% cpu and does not continue => I suspect an endless loop there.

Going into debug mode I end in:

DEBUG: recomy-staging.panter.ch: Finishing run of __directory/home/rails/app/releases
DEBUG: recomy-staging.panter.ch: Trying to run object __panter_rails_railshome/singleton
INFO: recomy-staging.panter.ch: Generating and executing code for __panter_rails_railshome/singleton
DEBUG: recomy-staging.panter.ch: Finishing run of __panter_rails_railshome/singleton

I'll probably dig into that tomorrow, but for now this is not yet ready to get merged.

@telmich
@telmich

Fixed in master

@telmich telmich closed this May 27, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment