kube-rs / kube

Rust Kubernetes client and controller runtime
https://kube.rs
Apache License 2.0
3.05k stars 318 forks source link

Add `find_all` method to `Store` #1634

Closed pando85 closed 1 week ago

pando85 commented 2 weeks ago

Resolves #1633.

Motivation

Solution

pando85 commented 2 weeks ago

I tried to return an Slice or an iterator but it requires a collect anyway because store is behind a RwLock.

codecov[bot] commented 2 weeks ago

Codecov Report

Attention: Patch coverage is 0% with 3 lines in your changes missing coverage. Please review.

Project coverage is 75.3%. Comparing base (9c402a6) to head (9f04a48). Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
kube-runtime/src/reflector/store.rs 0.0% 3 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #1634 +/- ## ======================================= - Coverage 75.3% 75.3% -0.0% ======================================= Files 82 82 Lines 7348 7351 +3 ======================================= Hits 5528 5528 - Misses 1820 1823 +3 ``` | [Files with missing lines](https://app.codecov.io/gh/kube-rs/kube/pull/1634?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kube-rs) | Coverage Δ | | |---|---|---| | [kube-runtime/src/reflector/store.rs](https://app.codecov.io/gh/kube-rs/kube/pull/1634?src=pr&el=tree&filepath=kube-runtime%2Fsrc%2Freflector%2Fstore.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kube-rs#diff-a3ViZS1ydW50aW1lL3NyYy9yZWZsZWN0b3Ivc3RvcmUucnM=) | `94.7% <0.0%> (-2.2%)` | :arrow_down: |
SOF3 commented 2 weeks ago

it requires a collect anyway because store is behind a RwLock.

can't you move the RwLockReadGuard into the returned iterator?

pando85 commented 2 weeks ago

I don't understand your point @SOF3 . Could you elaborate on it a bit?

SOF3 commented 1 week ago

I am just confused why we need so many special methods on Store instead of exposing a map-like API that provides iter(). Is it simply because we don't want to implement a custom Iterator type that owns its own RwLockReadGuard?

pando85 commented 1 week ago

@SOF3 that's a good point. In deed, we already have the state method which returns an iterator through all the objects.

I can close this and use that method.

Thank you both.