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

Refactor/collections init operations #548

Merged
merged 7 commits into from
Feb 20, 2024

Conversation

PaulFarault
Copy link
Contributor

Which issue(s) this PR fixes

Fixes None

Additional comments

This PR addresses the issue with assigning collection_name values. Previously, the assigned collection_name was that of the first collection in which the operation was specified in the DAG. Now, the collection_name for a playbook is the one in which the playbook is defined.

Another key benefit of this PR is the decoupling of the Operation creation logic. Playbook operations are now exclusively forged within Collection, and Collections is limited to forging Noop operations only.

I've intentionally broken down the PR into numerous commits to ease the review process. The most significant logic change is found in the "refactor: create playbook operations in Collection" commit.

Agreements

@PaulFarault PaulFarault requested a review from rpignolet February 9, 2024 10:33
PaulFarault and others added 3 commits February 12, 2024 14:51
Inventory reader should be passed directly so that it is used by the Collection __init__
@PaulFarault PaulFarault force-pushed the refactor/collections-init-operations branch from 3161414 to ede2005 Compare February 12, 2024 15:51
tdp/core/collections.py Outdated Show resolved Hide resolved
tdp/core/deployment/deployment_runner.py Outdated Show resolved Hide resolved
@PaulFarault PaulFarault force-pushed the refactor/collections-init-operations branch from 1dcc3a1 to cad3850 Compare February 20, 2024 10:41
@PaulFarault
Copy link
Contributor Author

@rpignolet I rolled back the Collection edition to focus on Collections initialization. I'll improve the Collection class in a following PR.

@PaulFarault PaulFarault force-pushed the refactor/collections-init-operations branch from cad3850 to 7914ccf Compare February 20, 2024 10:47
Copy link
Contributor

@SteBaum SteBaum left a comment

Choose a reason for hiding this comment

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

tested it again and it works

@PaulFarault PaulFarault force-pushed the refactor/collections-init-operations branch from 1a20184 to d52fe14 Compare February 20, 2024 13:34
@PaulFarault
Copy link
Contributor Author

PaulFarault commented Feb 20, 2024

Force pushed after squashing and rebasing.

@PaulFarault PaulFarault merged commit ed8a40f into master Feb 20, 2024
5 checks passed
@PaulFarault PaulFarault deleted the refactor/collections-init-operations branch February 20, 2024 13:38
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