Skip to content

ROX-28697: use Walk instead of GetAll in logimbue and policycategory#14742

Merged
janisz merged 2 commits intomasterfrom
use_walk_inseatd_of_getall_in_logs
Apr 4, 2025
Merged

ROX-28697: use Walk instead of GetAll in logimbue and policycategory#14742
janisz merged 2 commits intomasterfrom
use_walk_inseatd_of_getall_in_logs

Conversation

@janisz
Copy link
Contributor

@janisz janisz commented Mar 25, 2025

Description

We are using GetAll in logimbue this is not optimal as we load all data into memory and then convert them to json and write to zip. With this change we can do it one by one in a stream.
This PR removes writer package as it's now part of the service as it was the only place it was used.


  • 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 force-pushed the use_walk_inseatd_of_getall_in_logs branch 2 times, most recently from 5b5956e to 1e43af3 Compare March 25, 2025 14:15
Signed-off-by: Tomasz Janiszewski <tomek@redhat.com>
@janisz janisz force-pushed the use_walk_inseatd_of_getall_in_logs branch from 1e43af3 to be6379e Compare March 25, 2025 14:22
@rhacs-bot
Copy link
Contributor

rhacs-bot commented Mar 25, 2025

Images are ready for the commit at 3c6be8e.

To use with deploy scripts, first export MAIN_IMAGE_TAG=4.8.x-320-g3c6be8ebd8.

@codecov
Copy link

codecov bot commented Mar 25, 2025

Codecov Report

Attention: Patch coverage is 72.00000% with 7 lines in your changes missing coverage. Please review.

Project coverage is 48.88%. Comparing base (8e68c6a) to head (3c6be8e).
Report is 69 commits behind head on master.

Files with missing lines Patch % Lines
central/policycategory/datastore/singleton.go 0.00% 4 Missing ⚠️
central/debug/service/service.go 85.71% 2 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master   #14742      +/-   ##
==========================================
- Coverage   49.20%   48.88%   -0.32%     
==========================================
  Files        2533     2546      +13     
  Lines      185529   186956    +1427     
==========================================
+ Hits        91287    91400     +113     
- Misses      87005    88312    +1307     
- Partials     7237     7244       +7     
Flag Coverage Δ
go-unit-tests 48.88% <72.00%> (-0.32%) ⬇️

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 force-pushed the use_walk_inseatd_of_getall_in_logs branch from 2331599 to 343de05 Compare March 26, 2025 10:10
@janisz janisz added the auto-retest PRs with this label will be automatically retested if prow checks fails label Mar 26, 2025
@janisz janisz changed the title fix(logimbue): use Walk instead of GetAll ROX-28697: use Walk instead of GetAll in logimbue Mar 26, 2025
@janisz janisz changed the title ROX-28697: use Walk instead of GetAll in logimbue ROX-28697: use Walk instead of GetAll in logimbue and policycategory Mar 26, 2025
Signed-off-by: Tomasz Janiszewski <tomek@redhat.com>
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.
Thanks for taking the initiative of this refactor.

@janisz janisz merged commit 6db52e7 into master Apr 4, 2025
93 checks passed
@janisz janisz deleted the use_walk_inseatd_of_getall_in_logs branch April 4, 2025 12:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants