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

fix: remove obselete flags from redis options #135

Merged
merged 1 commit into from
May 13, 2024

Conversation

KevFan
Copy link
Contributor

@KevFan KevFan commented May 13, 2024

With https://github.com/Kuadrant/limitador/pull/317/files being merged, the following integration test has been failing due to the flags have been removed and no longer accepted by the latest version of Limitador

  [TIMEDOUT] Limitador controller Deploying limitador object with redis cached storage [It] with all the optional parameters, the command line is correct

Failing test in pipeline:

@KevFan KevFan requested a review from a team May 13, 2024 09:54
@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 79.61%. Comparing base (0713794) to head (74488f8).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #135      +/-   ##
==========================================
- Coverage   85.65%   79.61%   -6.05%     
==========================================
  Files          19       19              
  Lines         990      986       -4     
==========================================
- Hits          848      785      -63     
- Misses         93      139      +46     
- Partials       49       62      +13     
Flag Coverage Δ
integration 79.61% <ø> (-0.59%) ⬇️
unit ?

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

Components Coverage Δ
api/v1alpha1 (u) 90.47% <ø> (-9.53%) ⬇️
pkg/helpers (u) 83.87% <ø> (ø)
pkg/log (u) 57.89% <ø> (-36.85%) ⬇️
pkg/reconcilers (u) 67.68% <ø> (-6.99%) ⬇️
pkg/limitador (u) 91.71% <ø> (-7.03%) ⬇️
controllers (i) 75.67% <ø> (-1.02%) ⬇️
pkg/upgrades 88.88% <ø> (ø)
Files Coverage Δ
api/v1alpha1/limitador_types.go 90.47% <ø> (-9.53%) ⬇️
pkg/limitador/redis_cache_storage_options.go 88.23% <ø> (-11.77%) ⬇️

... and 8 files with indirect coverage changes

Copy link
Member

@alexsnaps alexsnaps left a comment

Choose a reason for hiding this comment

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

Thanks for this @KevFan ... I probably should have done this quicker, i.e. before anyone hits the issue indeed when running latest, my bad. This additional change will probably be needed to be added and I wasn't sure if anything then in turns affects the Kuadrant CR or if we just pass it "blindly" along... /cc @eguzki

@KevFan KevFan merged commit 60669ce into Kuadrant:main May 13, 2024
10 checks passed
@eguzki eguzki added the kind/breaking-change not backward compatible label May 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/breaking-change not backward compatible
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants