Skip to content

Conversation

@carlotaarvela
Copy link
Contributor

@carlotaarvela carlotaarvela commented Oct 24, 2025

Description

Allow user to download all captures from a specific namespace or from all namespaces

Related Issue

#1836

Checklist

  • I have read the contributing documentation.
  • I signed and signed-off the commits (git commit -S -s ...). See this documentation on signing commits.
  • I have correctly attributed the author(s) of the code.
  • I have tested the changes locally.
  • I have followed the project's style guidelines.
  • I have updated the documentation, if necessary.
  • I have added tests, if applicable.

Screenshots (if applicable) or Testing Completed

Please add any relevant screenshots or GIFs to showcase the changes made.

Additional Notes

Add any additional notes or context about the pull request here.


Please refer to the CONTRIBUTING.md file for more information on how to contribute to this project.

@carlotaarvela carlotaarvela requested a review from a team as a code owner October 24, 2025 13:59
downloadServices := make(map[string]*DownloadService)

// Collect all files in memory
allFiles := make(map[string][]byte)
Copy link
Contributor

Choose a reason for hiding this comment

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

Would this cause memory issues?
If captures files are large and there are multiple in different namespaces

if downloadAllNamespaces {
fmt.Println("Downloading all captures from all namespaces...")
} else {
fmt.Println("Downloading all captures...")
Copy link
Contributor

Choose a reason for hiding this comment

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

nit - add 'for namespace x' here

return nil
}

func downloadAllCaptures(ctx context.Context, config *rest.Config, namespace string) error {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit:
recommend breaking this down into a couple functions,

}

// Create the final archive
timestamp := time.Now().Format("20060102150405")
Copy link
Contributor

Choose a reason for hiding this comment

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

looks like a magic number - there should be constant for this in the time package


downloadCapture.Flags().StringVar(&blobURL, "blob-url", "", "Blob URL from which to download")
downloadCapture.Flags().StringVar(&captureName, "name", "", "The name of a the capture")
downloadCapture.Flags().BoolVar(&downloadAll, "all", false, "Download all available captures")
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 a bit unclear but looks like this is download all for specific namespace
We should clarify that so it's apparent the different between all and all-namespaces

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