Skip to content

ROX-28697: use Walk instead of GetAll in notifier#14763

Merged
janisz merged 7 commits intomasterfrom
ROX-15902_use_walk_in_notifier
Apr 10, 2025
Merged

ROX-28697: use Walk instead of GetAll in notifier#14763
janisz merged 7 commits intomasterfrom
ROX-15902_use_walk_in_notifier

Conversation

@janisz
Copy link
Contributor

@janisz janisz commented Mar 26, 2025 •
edited
Loading

Description

  • CHANGELOG update is not needed
  • Documentation is not needed

Testing

  • inspected CI results

Automated testing

  • modified existing tests
  • contributed no automated tests

How I validated my change

CI

@janisz janisz requested a review from a team as a code owner March 26, 2025 14:02
@rhacs-bot
Copy link
Contributor

rhacs-bot commented Mar 26, 2025 •
edited
Loading

Images are ready for the commit at f602835.

To use with deploy scripts, first export MAIN_IMAGE_TAG=4.8.x-325-gf60283512b.

@janisz
ROX-28697: use Walk instead of GetAll in notifier
0749873 
Signed-off-by: Tomasz Janiszewski <tomek@redhat.com>
@janisz janisz force-pushed the ROX-15902_use_walk_in_notifier branch from 8f02571 to 0749873 Compare March 27, 2025 16:53
This was referenced Mar 27, 2025
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
rhybrillou
rhybrillou reviewed Apr 4, 2025
View reviewed changes
@janisz
fixes
92ffaae 
Signed-off-by: Tomasz Janiszewski <tomek@redhat.com>
@openshift-ci
Copy link

openshift-ci bot commented Apr 7, 2025 •
edited
Loading

@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-17-compliance-e2e-tests 0749873 link false /test ocp-4-17-compliance-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 added 2 commits April 8, 2025 11:57
@janisz
fix
8c98670 
Signed-off-by: Tomasz Janiszewski <tomek@redhat.com>
@janisz
fix
095e931 
Signed-off-by: Tomasz Janiszewski <tomek@redhat.com>
@janisz janisz requested a review from rhybrillou April 8, 2025 10:32
@janisz janisz added the auto-retest PRs with this label will be automatically retested if prow checks fails label Apr 8, 2025
@codecov
Copy link

codecov bot commented Apr 8, 2025 •
edited
Loading

Codecov Report

Attention: Patch coverage is 35.59322% with 38 lines in your changes missing coverage. Please review.

Project coverage is 48.96%. Comparing base (8e68c6a) to head (f602835).
Report is 130 commits behind head on master.

Files with missing lines Patch % Lines
central/notifier/datastore/datastore_impl.go 40.00% 8 Missing and 1 partial ⚠️
central/notifier/datastore/telemetry.go 0.00% 8 Missing ⚠️
central/graphql/resolvers/notifiers.go 0.00% 6 Missing ⚠️
central/compliance/data/repository.go 0.00% 5 Missing ⚠️
central/notifier/processor/singleton.go 0.00% 5 Missing ⚠️
central/notifier/service/service_impl.go 0.00% 5 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##           master   #14763      +/-   ##
==========================================
- Coverage   49.20%   48.96%   -0.25%     
==========================================
  Files        2533     2550      +17     
  Lines      185529   187254    +1725     
==========================================
+ Hits        91287    91681     +394     
- Misses      87005    88322    +1317     
- Partials     7237     7251      +14
Flag Coverage Δ
go-unit-tests 48.96% <35.59%> (-0.25%) ⬇️

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 janisz requested a review from parametalol April 9, 2025 15:54
rhybrillou
rhybrillou reviewed Apr 9, 2025
View reviewed changes
janisz and others added 3 commits April 9, 2025 19:36
@janisz @rhybrillou
Update central/notifier/service/service_impl.go
017beb8 
Co-authored-by: Yann Brillouet <91869377+rhybrillou@users.noreply.github.com>
@janisz
fix
88ffd7e 
Signed-off-by: Tomasz Janiszewski <tomek@redhat.com>
@janisz
add e2e test
f602835 
Signed-off-by: Tomasz Janiszewski <tomek@redhat.com>
@janisz janisz requested a review from rhybrillou April 9, 2025 17:57
rhybrillou
rhybrillou approved these changes Apr 10, 2025
View reviewed changes
Copy link
Contributor

@rhybrillou rhybrillou left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you.

@janisz janisz enabled auto-merge (squash) April 10, 2025 07:45
@janisz janisz disabled auto-merge April 10, 2025 07:45
@janisz janisz enabled auto-merge (squash) April 10, 2025 07:45
@janisz janisz disabled auto-merge April 10, 2025 08:02
@janisz janisz merged commit 8e31f7d into master Apr 10, 2025
88 of 89 checks passed
@janisz janisz deleted the ROX-15902_use_walk_in_notifier branch April 10, 2025 09:06
janisz added a commit that referenced this pull request Apr 14, 2025
@janisz @rhybrillou
ROX-28697: use Walk instead of GetAll in notifier (#14763)
966b225 
Signed-off-by: Tomasz Janiszewski <tomek@redhat.com>
Co-authored-by: Yann Brillouet <91869377+rhybrillou@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rhybrillou rhybrillou rhybrillou approved these changes

@parametalol parametalol Awaiting requested review from parametalol

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 @rhybrillou