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

Improve dag parsing logic #628

Merged
merged 14 commits into from
Nov 14, 2024
Merged

Conversation

PaulFarault
Copy link
Contributor

@PaulFarault PaulFarault commented Oct 22, 2024

Which issue(s) this PR fixes

Fixes None

Additional comments

  • Isolate the DAG parsing logic from the collection module.
  • Collection.dag_nodes now returns a generator.

Agreements

@PaulFarault PaulFarault requested a review from SteBaum October 22, 2024 16:16
@PaulFarault PaulFarault self-assigned this Oct 22, 2024
@PaulFarault PaulFarault force-pushed the collection-return-dag-generator branch from c3f21aa to b3c28bb Compare October 22, 2024 16:29
@PaulFarault PaulFarault force-pushed the collection-return-dag-generator branch 2 times, most recently from 2a14a35 to c358d8d Compare October 22, 2024 20:50
@PaulFarault PaulFarault marked this pull request as draft October 23, 2024 09:52
@PaulFarault PaulFarault force-pushed the collection-return-dag-generator branch from c358d8d to f57c27f Compare October 23, 2024 10:08
@PaulFarault PaulFarault force-pushed the collection-return-dag-generator branch 2 times, most recently from 006fe09 to effa3d8 Compare October 31, 2024 08:48
@PaulFarault
Copy link
Contributor Author

PaulFarault commented Oct 31, 2024

This new version transforms the Collection class to CollectionReader.

The CollectionReader class doesn't store data anymore, only a few metadata remain (collection name, path and paths to directories). This enable to strictly restrict the use of CollectionReader to Collections (that uses it to initialize itself).

To accomplish that:

  • I introduced 3 methods that directly read from the collection on call : read_dag_nodes, read_playbooks and read_schemas and deleted the related @property ;

  • I added new properties to the Collections class so that it can hold all collections informations (that were previously retrieved from Collection):

    • playbooks a dictionary of available playbooks.
    • default_vars_dirs a dictionary of each collection default var directory path.
    • hostable_entities a mapping of service names to a set of their components.

    This last addition allows to remove the Collections.get_components_from_service method. Collections now only exposes @property.

  • Finally, I changed the Collections definition so that it is not a mapping of collection name to Collection instance anymore.

In addition, Collections and CollectionReader has been move to a tdp.core.collections package that only exports the Collections class and a few Error that can be raised. Methods used by the readers methods from the CollectionReader class has been extracted into their own files and unit tests have been refactored accordingly.

@PaulFarault PaulFarault force-pushed the collection-return-dag-generator branch from effa3d8 to 1f96afd Compare October 31, 2024 09:34
@PaulFarault PaulFarault marked this pull request as ready for review October 31, 2024 09:38
@PaulFarault PaulFarault requested a review from rpignolet October 31, 2024 09:38
@PaulFarault PaulFarault force-pushed the collection-return-dag-generator branch from 1f96afd to 65ec5be Compare November 7, 2024 10:09
@PaulFarault PaulFarault force-pushed the collection-return-dag-generator branch from 41349da to f3516e8 Compare November 7, 2024 10:39
@PaulFarault PaulFarault merged commit 8d7c668 into master Nov 14, 2024
5 checks passed
@PaulFarault PaulFarault deleted the collection-return-dag-generator branch November 14, 2024 08:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants