-
Notifications
You must be signed in to change notification settings - Fork 39
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
model consumption in path expansion #776
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -4,6 +4,7 @@ import ( | |||||||||||||||||||
"errors" | ||||||||||||||||||||
"fmt" | ||||||||||||||||||||
"reflect" | ||||||||||||||||||||
"strings" | ||||||||||||||||||||
|
||||||||||||||||||||
"github.com/dominikbraun/graph" | ||||||||||||||||||||
"github.com/klothoplatform/klotho/pkg/collectionutil" | ||||||||||||||||||||
|
@@ -64,6 +65,11 @@ func checkProperties(ctx solution_context.SolutionContext, resource, toCheck *co | |||||||||||||||||||
if err != nil || template == nil { | ||||||||||||||||||||
return false, fmt.Errorf("error getting resource template for resource %s: %w", resource.ID, err) | ||||||||||||||||||||
} | ||||||||||||||||||||
|
||||||||||||||||||||
if strings.Contains(resource.ID.Name, PHANTOM_PREFIX) { | ||||||||||||||||||||
return true, nil | ||||||||||||||||||||
} | ||||||||||||||||||||
|
||||||||||||||||||||
explicitlyNotValid := false | ||||||||||||||||||||
explicitlyValid := false | ||||||||||||||||||||
err = template.LoopProperties(resource, func(prop *knowledgebase.Property) error { | ||||||||||||||||||||
|
@@ -95,7 +101,7 @@ func checkProperties(ctx solution_context.SolutionContext, resource, toCheck *co | |||||||||||||||||||
return knowledgebase.ErrStopWalk | ||||||||||||||||||||
} | ||||||||||||||||||||
} else { | ||||||||||||||||||||
loneDep, err := checkIfLoneDependency(ctx, resource.ID, toCheck.ID, direction) | ||||||||||||||||||||
loneDep, err := checkIfLoneDependency(ctx, resource.ID, toCheck.ID, direction, selector) | ||||||||||||||||||||
if err != nil { | ||||||||||||||||||||
return err | ||||||||||||||||||||
} | ||||||||||||||||||||
|
@@ -155,7 +161,9 @@ func checkIfPropertyContainsResource(property interface{}, resource construct.Re | |||||||||||||||||||
return false | ||||||||||||||||||||
} | ||||||||||||||||||||
|
||||||||||||||||||||
func checkIfLoneDependency(ctx solution_context.SolutionContext, resource, toCheck construct.ResourceId, direction knowledgebase.Direction) (bool, error) { | ||||||||||||||||||||
func checkIfLoneDependency(ctx solution_context.SolutionContext, | ||||||||||||||||||||
resource, toCheck construct.ResourceId, direction knowledgebase.Direction, | ||||||||||||||||||||
selector knowledgebase.ResourceSelector) (bool, error) { | ||||||||||||||||||||
Comment on lines
+164
to
+166
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
nit: I prefer this style of multi-line definition, but we haven't standardized anything |
||||||||||||||||||||
var resources []construct.ResourceId | ||||||||||||||||||||
var err error | ||||||||||||||||||||
// we are going to check if the resource was created as a unique resource by any of its direct dependents. if it was and that | ||||||||||||||||||||
|
@@ -177,8 +185,24 @@ func checkIfLoneDependency(ctx solution_context.SolutionContext, resource, toChe | |||||||||||||||||||
return true, nil | ||||||||||||||||||||
} else if len(resources) == 1 && resources[0].Matches(toCheck) { | ||||||||||||||||||||
return true, nil | ||||||||||||||||||||
} else { | ||||||||||||||||||||
for _, res := range resources { | ||||||||||||||||||||
depRes, err := ctx.RawView().Vertex(res) | ||||||||||||||||||||
if err != nil { | ||||||||||||||||||||
return false, err | ||||||||||||||||||||
} | ||||||||||||||||||||
data := knowledgebase.DynamicValueData{Resource: resource} | ||||||||||||||||||||
dynCtx := solution_context.DynamicCtx(ctx) | ||||||||||||||||||||
canUse, err := selector.CanUse(dynCtx, data, depRes) | ||||||||||||||||||||
if err != nil { | ||||||||||||||||||||
return false, err | ||||||||||||||||||||
} | ||||||||||||||||||||
if canUse { | ||||||||||||||||||||
return false, nil | ||||||||||||||||||||
} | ||||||||||||||||||||
} | ||||||||||||||||||||
return true, nil | ||||||||||||||||||||
} | ||||||||||||||||||||
return false, nil | ||||||||||||||||||||
} | ||||||||||||||||||||
|
||||||||||||||||||||
// checkIfCreatedAsUnique checks if the resource was created as a unique resource by any of its direct dependents. if it was and that | ||||||||||||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,6 +7,7 @@ import ( | |
"strings" | ||
|
||
"github.com/dominikbraun/graph" | ||
"github.com/klothoplatform/klotho/pkg/collectionutil" | ||
construct "github.com/klothoplatform/klotho/pkg/construct2" | ||
"github.com/klothoplatform/klotho/pkg/engine2/operational_eval" | ||
"github.com/klothoplatform/klotho/pkg/engine2/path_selection" | ||
|
@@ -126,25 +127,27 @@ func (e *Engine) GetViewsDag(view View, ctx solution_context.SolutionContext) (c | |
dstTag := e.GetResourceVizTag(string(DataflowView), dst) | ||
switch dstTag { | ||
case ParentIconTag: | ||
hasPath, err := HasParent(topo, ctx, src, dst) | ||
if err != nil { | ||
errs = errors.Join(errs, err) | ||
} | ||
if node.Parent.IsZero() && hasPath { | ||
node.Parent = dst | ||
} | ||
case BigIconTag: | ||
hasPath, err := HasPath(topo, ctx, src, dst) | ||
if err != nil { | ||
errs = errors.Join(errs, err) | ||
} | ||
if hasPath { | ||
edge := construct.SimpleEdge{ | ||
Source: src, | ||
Target: dst, | ||
if node.Parent.IsZero() { | ||
template, err := e.Kb.GetResourceTemplate(dst) | ||
if err != nil { | ||
errs = errors.Join(errs, err) | ||
continue | ||
} | ||
if collectionutil.Contains(template.Classification.Is, "cluster") || | ||
collectionutil.Contains(template.Classification.Is, "network") { | ||
hasPath, err := HasParent(topo, ctx, src, dst) | ||
if err != nil { | ||
errs = errors.Join(errs, err) | ||
} | ||
if node.Parent.IsZero() && hasPath { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
node.Parent = dst | ||
} | ||
} else { | ||
errs = errors.Join(errs, createEdgeIfPath(topo, ctx, src, dst, path)) | ||
} | ||
topo.Edges[edge] = path[1 : len(path)-1] | ||
} | ||
case BigIconTag: | ||
errs = errors.Join(errs, createEdgeIfPath(topo, ctx, src, dst, path)) | ||
case SmallIconTag: | ||
if seenSmall.Contains(dst) { | ||
continue | ||
|
@@ -231,6 +234,25 @@ func (e *Engine) GetViewsDag(view View, ctx solution_context.SolutionContext) (c | |
return viewDag, nil | ||
} | ||
|
||
func createEdgeIfPath(topo Topology, | ||
ctx solution_context.SolutionContext, | ||
src, dst construct.ResourceId, | ||
path construct.Path, | ||
) error { | ||
hasPath, err := HasPath(topo, ctx, src, dst) | ||
if err != nil { | ||
return err | ||
} | ||
if hasPath { | ||
edge := construct.SimpleEdge{ | ||
Source: src, | ||
Target: dst, | ||
} | ||
topo.Edges[edge] = path[1 : len(path)-1] | ||
} | ||
return nil | ||
} | ||
|
||
func (e *Engine) getParentFromNamespace(resource construct.ResourceId, resources []construct.ResourceId) construct.ResourceId { | ||
if resource.Namespace != "" { | ||
for _, potentialParent := range resources { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
HasPrefix
is technically more correct, right?