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 OpenRelik File Download Handling and Folder Management #965

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

jleaniz
Copy link
Collaborator

@jleaniz jleaniz commented Jan 29, 2025

Summary:

This PR enhances the OpenRelik processor by improving error handling during file downloads and streamlining folder management within OpenRelik. It also updates several dependencies.

Technical Details:

  • Improved Download Error Handling: The DownloadWorkflowOutput function in dftimewolf/lib/processors/openrelik.py now uses the dedicated openrelik_api_client.download_file function and explicitly handles cases where the download fails. It logs an error message and returns None if the download is unsuccessful, preventing downstream errors and providing better feedback to the user.
  • Streamlined Folder Management: The OpenRelik processor now reuses the initially created folder ID for subsequent file uploads and workflow execution. This prevents the creation of multiple folders per incident and simplifies navigation within OpenRelik. The folder is also updated with the incident ID as its display name, improving organization and clarity.
  • Dependency Updates: Several dependencies have been updated to their latest versions to benefit from bug fixes, performance improvements, and new features. This includes Poetry, which is now at version 1.8.5, reflecting our commitment to keeping our toolchain current. Notably, grpcio, numpy, pandas, and several Google Cloud libraries were updated, potentially improving performance and security.
  • Type Hinting: The return type of DownloadWorkflowOutput has been updated to str | None to reflect the possible return of None in case of download failure, enhancing type safety.

@jleaniz jleaniz requested a review from tomchop January 29, 2025 19:38

self.logger.info(f"Created folder {self.folder_id}")
self.logger.info(f"Updating folder {self.folder_id}")
_ = self.openrelik_folder_client.update_folder(
Copy link
Collaborator

Choose a reason for hiding this comment

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

You can skip the empty assignment here

Comment on lines +130 to +136
if not self.folder_id or not self.openrelik_folder_client.folder_exists(
self.folder_id
):
folder_id = self.openrelik_folder_client.create_root_folder(
self.folder_id = self.openrelik_folder_client.create_root_folder(
f"{self.incident_id}"
)
self.logger.info(f"Created folder {folder_id}")

self.logger.info(f"Created folder {self.folder_id}")
Copy link
Collaborator

Choose a reason for hiding this comment

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

If we're using the same folder for all files, wouldn't we want this piece of logic to go in SetUp? It looks like there would be a race condition if multiple threads ran at the sam time.

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