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

72 artifactless propagation performance optimization #75

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

iignatevich
Copy link
Collaborator

No description provided.

@iignatevich iignatevich force-pushed the 72-artifactless-propagation-performance-optimization branch 4 times, most recently from d90b1cf to 146703c Compare January 20, 2025 14:45
title: Use playbook resources
description: Parse playbooks and propagate used resources only
type: boolean
default: false
Copy link
Contributor

@davidferlay davidferlay Jan 20, 2025

Choose a reason for hiding this comment

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

Probably should be true by default

title: Commits after
description: Use commits only after specific date
type: string
default: ""
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we could find a more intuitive naming for this option, somrhing like commit-depth or time-depth

@iignatevich iignatevich force-pushed the 72-artifactless-propagation-performance-optimization branch from 146703c to 8b35dc1 Compare January 20, 2025 15:49
@iignatevich iignatevich force-pushed the 72-artifactless-propagation-performance-optimization branch from 6674aae to f2f1a13 Compare January 23, 2025 14:05
@@ -61,7 +61,10 @@ type Inventory struct {
dependsOn map[string]*OrderedMap[bool]
topOrder []string

variablesCalculated bool
resourcesUsageCalculated bool
usedResources map[string]bool
Copy link
Contributor

@davidferlay davidferlay Jan 24, 2025

Choose a reason for hiding this comment

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

➜  ski-platform git:(master) pl deps integration/libraries/roles/loopback/meta/plasma.yaml                                                                                                     
 INFO  Selected source is .compose/build
 INFO  Dependent resources:
platform/libraries/roles/executor
 INFO  Dependencies:
integration/libraries/roles/errors
integration/libraries/roles/metrics
integration/libraries/roles/utils
platform/libraries/roles/configuration
platform/libraries/roles/event
platform/libraries/roles/log
platform/libraries/roles/transport
INFO  Dependent resources:
platform/libraries/roles/executor

So according to deps command, platform/libraries/roles/executor does not seem to be used in any other resource, hence integration/libraries/roles/loopback is not propagated

Whereas platform/libraries/roles/executor does seem to be used in integration/helpers/roles/executor_builder

➜  ski-platform git:(master) cat .compose/build/integration/helpers/roles/executor_builder/tasks/dependencies.yaml | grep "Load platform.libraries.executor" -A4
- name: Load platform.libraries.executor dependency
  include_role:
    name: platform.libraries.executor
  when: library_tag in platform__libraries__executor.mrt and platform__libraries__executor.state[library_tag]['build']

Issue seems to be that in some cases, like helpers, depdency is not listed in /tasks/dependencies.yaml but in /tasks/main.yaml

So we could change dependency parser logic to look for include_role: in all /tasks/*.yaml or both /tasks/dependencies.yaml and /tasks/main.yaml instead of only looking in /tasks/dependencies.yaml as it's the case today

By doing that, all resources should be added to propag list and their deps propagated, at the exception of helpers


In addition of that, maybe need to change how is_valid_resource is determined: I guess all resource types should be added to dependency tree, list of valid resource types should only be used to know if version of these resources should be updated by sync or not, which happens later in logic

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.

2 participants