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

[Dumper] Rework of the object ingestion #174

Merged
merged 10 commits into from
Mar 18, 2024

Conversation

jt-dd
Copy link
Contributor

@jt-dd jt-dd commented Mar 18, 2024

Create an interface for each object type to add the ability to create an objectList for each k8s type. For instance all endpoint stack them into a discoveryv1.EndpointSliceList. This is needed by the collector.
Effort were already made to be fully supported by the writers (tar and file one).

@jt-dd jt-dd requested a review from a team as a code owner March 18, 2024 12:52
edznux-dd
edznux-dd previously approved these changes Mar 18, 2024
Copy link
Contributor

@edznux-dd edznux-dd left a comment

Choose a reason for hiding this comment

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

Assuming the pkg/dump/ingestor has commented out part because it's for the next PR, LGTM! (will need to fix the linter for those commented out part)

Comment on lines +87 to +92
// err = pipeline.Run(ctx)
// if err != nil {
// return fmt.Errorf("run pipeline ingestor: %w", err)
// }

return nil
// return pipeline.Wait(ctx)
Copy link
Contributor

Choose a reason for hiding this comment

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

This is commented for the next PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes it is. to have a "pain less" split.

@jt-dd jt-dd merged commit f863374 into main Mar 18, 2024
7 checks passed
@jt-dd jt-dd deleted the jt-dd/rework-object-ingesting-dumping branch March 18, 2024 16:44
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