Skip to content

Aggregated command api module #40731

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 54 commits into
base: master
Choose a base branch
from
Open

Aggregated command api module #40731

wants to merge 54 commits into from

Conversation

YuvHayun
Copy link
Contributor

@YuvHayun YuvHayun commented Jul 27, 2025 •
edited
Loading

Status

  • In Progress
  • Ready
  • In Hold - (Reason for hold)

Related Issues

fixes: CRTX-187725

Description

A pr for implementing the aggregated command API module.

@github-actions GitHub Actions
Copy link

github-actions bot commented Jul 27, 2025 •
edited
Loading

Coverage

Coverage Report
FileStmtsMissCoverMissing
Packs/AggregatedScripts/Scripts/CVEEnrichment
   CVEEnrichment.py170100% 
Packs/AggregatedScripts/Scripts/DomainEnrichment
   DomainEnrichment.py190100% 
Packs/AggregatedScripts/Scripts/IPEnrichment
   IPEnrichment.py321262%87–94, 96–99
Packs/AggregatedScripts/Scripts/URLEnrichment
   URLEnrichment.py180100% 
Packs/ApiModules/Scripts/AggregatedCommandApiModule
   AggregatedCommandApiModule.py4508182%150–153, 197–201, 470–471, 474–479, 481–482, 485–488, 494–495, 498–499, 502–503, 506–507, 631–632, 649, 657, 681–685, 687–693, 695–696, 699–701, 703, 705–706, 708–713, 715, 721, 765–766, 774–775, 868–870, 872–875, 877–878, 903–904, 920–921, 1108
TOTAL5369382% 

Tests Skipped Failures Errors Time
116 0 💤 0 ❌ 0 🔥 4.765s ⏱️

YuvHayun
YuvHayun commented Aug 5, 2025
View reviewed changes
YuvHayun
YuvHayun commented Aug 7, 2025
View reviewed changes
Copy link
Contributor Author

@YuvHayun YuvHayun left a comment

Choose a reason for hiding this comment

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

Very nice!
Let's try to rearrange the code such that it'll also inflect the logical flow (i.e, move BatchExecutor close to Command, etc..)

YuvHayun
YuvHayun commented Aug 10, 2025
View reviewed changes
@almog2296 almog2296 requested a review from Shellyber August 11, 2025 11:34
JudahSchwartz
JudahSchwartz reviewed Aug 14, 2025
View reviewed changes
Copy link
Contributor

@JudahSchwartz JudahSchwartz left a comment

Choose a reason for hiding this comment

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

great code!

@content-bot
Copy link
Collaborator

⚠️ The PR is missing the ready-for-pipeline-running label. Please add the label when the PR is ready in order to proceed.

@content-bot
Copy link
Collaborator

Validate summary
The following errors were thrown as a part of this pr: BA127, RN106, BA103, PA114.
The following errors can be ignored: BA127.
The following errors cannot be ignored: RN106, BA103, PA114.
If the AG100 validation in the pre-commit GitHub Action fails, the pull request cannot be force-merged.
The following errors don't run as part of the nightly flow and therefore can be force merged: BA127, RN106, BA103, PA114.

Verdict: PR can be force merged from validate perspective? ✅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JudahSchwartz JudahSchwartz JudahSchwartz left review comments

@joeymizrahi joeymizrahi joeymizrahi left review comments

@almog2296 almog2296 almog2296 left review comments

@Shellyber Shellyber Awaiting requested review from Shellyber

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@YuvHayun @content-bot @JudahSchwartz @joeymizrahi @almog2296