Skip to content

fix(processbaseline): use GetByQueryFn#16991

Open
janisz wants to merge 3 commits intomasterfrom
use-get-by-query-fn
Open

fix(processbaseline): use GetByQueryFn#16991
janisz wants to merge 3 commits intomasterfrom
use-get-by-query-fn

Conversation

@janisz
Copy link
Contributor

@janisz janisz commented Sep 23, 2025 •
edited
Loading

Replace SearchRawProcessIndicators with GetByQueryFn in processbaseline datastore where we only iterate over process indicators. This improves memory efficiency by processing indicators individually rather than collecting them into a slice.

🤖 Generated with Claude Code

Co-Authored-By: Claude noreply@anthropic.com

@janisz
Copy link
Contributor Author

janisz commented Sep 23, 2025 •
edited
Loading

This change is part of the following stack:

Change managed by git-spice.

@janisz janisz mentioned this pull request Sep 23, 2025
Closed
@janisz janisz force-pushed the use-get-by-query-fn branch from 2d9e773 to 0f9274f Compare September 23, 2025 17:08
@janisz janisz mentioned this pull request Sep 24, 2025
Merged
@janisz janisz force-pushed the optimize_reprocess_deployment_risk branch from b515b59 to 2078c69 Compare September 24, 2025 11:20
@janisz janisz force-pushed the use-get-by-query-fn branch from 0f9274f to 4a73d96 Compare September 24, 2025 11:20
@janisz janisz requested review from mtodor and rhybrillou September 24, 2025 11:21
@rhacs-bot
Copy link
Contributor

rhacs-bot commented Sep 24, 2025 •
edited
Loading

Images are ready for the commit at dbab594.

To use with deploy scripts, first export MAIN_IMAGE_TAG=4.11.x-432-gdbab5945c3.

dashrews78
dashrews78 reviewed Sep 29, 2025
View reviewed changes
@janisz janisz changed the base branch from optimize_reprocess_deployment_risk to master October 2, 2025 13:11
@janisz janisz force-pushed the use-get-by-query-fn branch 3 times, most recently from c61be92 to 73736bc Compare October 7, 2025 07:19
@codecov
Copy link

codecov bot commented Oct 7, 2025 •
edited
Loading

Codecov Report

❌ Patch coverage is 23.80952% with 32 lines in your changes missing coverage. Please review.
✅ Project coverage is 49.27%. Comparing base (123b457) to head (dbab594).
⚠️ Report is 17 commits behind head on master.

Files with missing lines Patch % Lines
central/detection/lifecycle/manager_impl.go 0.00% 17 Missing ⚠️
central/graphql/resolvers/deploymentevents.go 0.00% 11 Missing ⚠️
...entral/processbaseline/datastore/datastore_impl.go 83.33% 1 Missing and 1 partial ⚠️
...ntral/processindicator/datastore/datastore_impl.go 0.00% 2 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##           master   #16991      +/-   ##
==========================================
+ Coverage   49.25%   49.27%   +0.01%     
==========================================
  Files        2735     2735              
  Lines      206138   206226      +88     
==========================================
+ Hits       101539   101623      +84     
- Misses      97051    97059       +8     
+ Partials     7548     7544       -4
Flag Coverage Δ
go-unit-tests 49.27% <23.80%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@janisz
Copy link
Contributor Author

janisz commented Oct 7, 2025

/retest

@stackrox stackrox deleted a comment from red-hat-konflux bot Oct 7, 2025
@janisz janisz requested a review from dashrews78 October 7, 2025 15:18
dashrews78
dashrews78 reviewed Oct 17, 2025
View reviewed changes
central/detection/lifecycle/manager_impl.go Outdated
query := search.NewQueryBuilder().
AddExactMatches(search.DeploymentID, deploymentID).
ProtoQuery()
_ = m.processesDataStore.GetByQueryFn(lifecycleMgrCtx, query, fn)
Copy link
Contributor

@dashrews78 dashrews78 Oct 17, 2025

Choose a reason for hiding this comment

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

Interesting we were squashing the error before. We should probably log it at least.

dashrews78
dashrews78 approved these changes Oct 17, 2025
View reviewed changes
Copy link
Contributor

@dashrews78 dashrews78 left a comment

Choose a reason for hiding this comment

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

left one nit about logging an error instead of squashing it.

janisz and others added 2 commits March 24, 2026 18:45
@janisz
fix(risk): improve reprocess deployment risk
32f8a0e 
Signed-off-by: Tomasz Janiszewski <tomek@redhat.com>

# Conflicts:
#	central/processbaseline/evaluator/evaluator_impl.go
#	central/processbaseline/evaluator/evaluator_test.go
@janisz @claude
fix(processbaseline): use GetByQueryFn
7767e39 
Replace SearchRawProcessIndicators with GetByQueryFn in processbaseline
datastore where we only iterate over process indicators. This improves
memory efficiency by processing indicators individually rather than
collecting them into a slice.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Tomasz Janiszewski <tomek@redhat.com>
@janisz janisz force-pushed the use-get-by-query-fn branch from 73736bc to 7767e39 Compare March 24, 2026 17:54
@openshift-ci
Copy link

openshift-ci bot commented Mar 24, 2026

@janisz: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/ocp-4-12-nongroovy-e2e-tests 7767e39 link false /test ocp-4-12-nongroovy-e2e-tests

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

@janisz janisz added the auto-retest PRs with this label will be automatically retested if prow checks fails label Mar 25, 2026
@janisz
Copy link
Contributor Author

janisz commented Mar 25, 2026

/retest

@janisz
utils.Should
dbab594 
Signed-off-by: Tomasz Janiszewski <tomek@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dashrews78 dashrews78 dashrews78 approved these changes

@mtodor mtodor Awaiting requested review from mtodor

@rhybrillou rhybrillou Awaiting requested review from rhybrillou

@JoukoVirtanen JoukoVirtanen Awaiting requested review from JoukoVirtanen

Assignees

No one assigned

Labels

area/central auto-retest PRs with this label will be automatically retested if prow checks fails

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@janisz @rhacs-bot @dashrews78