Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/requirements #54

Merged
merged 30 commits into from
Jun 8, 2016
Merged

Feature/requirements #54

merged 30 commits into from
Jun 8, 2016

Conversation

Dacode45
Copy link
Contributor

@Dacode45 Dacode45 commented Jun 7, 2016

Fulfills the Requirements issue.

Resource blocks should have requirements.

These requirements:

are limited to the names in the current module. You can require a module call to be completed, though.
If no requirements are specified, it is assumed that the task before the current task is the only requirement
If an empty list of tasks is specified, it is accepted that the task has no dependencies aside from being executed as part of the module
Caveats

Extended the Resource Interface Depends() []string -Extended the Task Interface with AddDep(string) and RemoveDep(string)
Implement the new/changed interface for relevant nodes (probably just ShellTask, Template, and ModuleCall)
Made sure walking the graph works correctly and in the order the new requirements specify.

langston-barrett and others added 16 commits May 26, 2016 17:09
- module arguments are now contained in a dedicated field 'params'
	ex module "x" "y"{
		params = {
			arg1 = "1"}}
- explicit dependencies now work with a few caveats
*an empty array of dependencies ([]) works but not an array of empty strings ([""])*
*dependent calls to a module task work fine*
Merge branch 'master' of https://github.com/asteris-llc/converge into feature/requirements
…module as a depedencies.

TODO : when we parallelize this, will this need to change?
@@ -67,8 +68,13 @@ func (g *Graph) load() error {
ids = append(ids, childID)
}
}
for _, id := range ids {
//fmt.Printf("ID: %q, Res: %+v\n\n", id.ID, id.Resource)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove comment

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@@ -0,0 +1,13 @@
param "message" { default = "Hello, World!" }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this file needs a better name. How about basicDependencies.hcl?

@@ -50,7 +50,8 @@ type ident struct {
}

func (g *Graph) load() error {
ids := []ident{{g.root.Name(), g.root}}
rootID := g.root.Name()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't need

David Ayeke and others added 4 commits June 7, 2016 14:12
1. Checks that dependencies of a resource are lower in the tree than the resource
2. Checks that terraform's tree goes through nodes depth first
The `TransitiveReduction` function was removing dependency links because
their goal is to make the graph equally as walkable but not necessarily
as connected. However, we still need the `b -> d` edge which was being
removed.
@BrianHicks
Copy link
Contributor

@Dacode45 check out b0d3e1c in the morning. The test had the correct order backwards, the leafmost node should be executed first, not the rootmost. I also simplified it quite a bit.

@BrianHicks BrianHicks merged commit 47b900b into master Jun 8, 2016
@BrianHicks BrianHicks deleted the feature/requirements branch June 8, 2016 05:03
@BrianHicks
Copy link
Contributor

That said, thanks for your hard work on this, it's great. 👍

BrianHicks added a commit that referenced this pull request Dec 22, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants