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

[Security Solution][Endpoint] Removes pMap and uses a for loop instead #163509

Conversation

dasansol92
Copy link
Contributor

@dasansol92 dasansol92 commented Aug 9, 2023

Summary

Removes pMap and uses a for loop instead when fetching exceptions (as it was done before) since exceptions are cached and there is no need for parallel code here.

Original PR with previous changes: #160387

@dasansol92 dasansol92 added release_note:skip Skip the PR/issue when compiling release notes Team:Defend Workflows “EDR Workflows” sub-team of Security Solution v8.10.0 labels Aug 9, 2023
@dasansol92 dasansol92 requested a review from a team as a code owner August 9, 2023 16:22
@dasansol92 dasansol92 requested review from joeypoon and tomsonpl August 9, 2023 16:22
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-defend-workflows (Team:Defend Workflows)

@dasansol92
Copy link
Contributor Author

@elasticmachine merge upstream

/** When set to false, instead of stopping when a promise rejects, it will wait for all the promises to
* settle and then reject with an aggregated error containing all the errors from the rejected promises. */
stopOnError: false,
for (const policyId of allPolicyIds)
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you know why we used pMap in the first place? The concurrency 5 value was for any particular reason?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The purpose of that was to decrease running time. 5 was the default value we used before in the clean function of the manifest manager, nothing specific. But since the API call responses are now cached, there is no need to parallelise this I guess. Also, it could end up doing extra API calls if the cache is not filled yet and two parallel executions try to fill it.

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

Copy link
Contributor

@tomsonpl tomsonpl left a comment

Choose a reason for hiding this comment

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

lgtm 👍

@dasansol92 dasansol92 merged commit 81ca020 into elastic:main Aug 11, 2023
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Aug 11, 2023
jloleysens added a commit to jloleysens/kibana that referenced this pull request Aug 11, 2023
* main: (64 commits)
  [ML] Transforms: Fix privileges check. (elastic#163687)
  [Log Explorer] Add test suite for Dataset Selector (elastic#163079)
  [Security Solution][Endpoint] Add API checks to Endpoint Policy create/update for checking `endpointPolicyProtections` is enabled (elastic#163429)
  [Security Solution] Fix flaky test: x-pack/test/detection_engine_api_integration/security_and_spaces/update_prebuilt_rules_package/update_prebuilt_rules_package·ts - update_prebuilt_rules_package should allow user to install prebuilt rules from scratch, then install new rules and upgrade existing rules from the new package (elastic#163241)
  [Security Solution] expandable flyout - replace feature flag with advanced settings toggle (elastic#161614)
  [DOCS] Adds the release notes for the 8.9.1 release. (elastic#163578)
  [FTR] Implement browser network condition utils (elastic#163633)
  [Security Solution] Unskip rules table auto-refresh Cypress tests (elastic#163451)
  [Security Solution] Re-enable fixed rule snoozing Cypress test (elastic#160037)
  [Flaky Test elastic#111821] Mock `moment` to avoid midnight TZ issues (elastic#163157)
  Document interactive setup (elastic#163619)
  [Lens] Align decoration color with text color for layer actions (elastic#163630)
  [Lens] Relax counter field checks for saved visualizations with unsupported operations (elastic#163515)
  [Security Solution][Endpoint] Removes pMap and uses a for loop instead (elastic#163509)
  [Enterprise Search] Update Workplace Search connectors doclink (elastic#163676)
  Update APM (main) (elastic#163623)
  [Serverless] Partially fix lens/maps/visualize breadcrumbs missing title  (elastic#163476)
  [Flaky elastic#118272] Unskip tests (elastic#163319)
  [APM] Make service group saved objects exportable (elastic#163569)
  [Observability AI Assistant] Action menu item (elastic#163463)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting release_note:skip Skip the PR/issue when compiling release notes Team:Defend Workflows “EDR Workflows” sub-team of Security Solution v8.10.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants