search

launchdarkly / rust-server-sdk

LaunchDarkly Server-Side SDK for Rust
https://docs.launchdarkly.com/sdk/server-side/rust
Other
18 stars 13 forks source link

Fix a bug in `HyperFeatureRequester::get_all()` #65

Closed aalexandrov closed 2 months ago

aalexandrov commented 3 months ago

The FeatureRequester::get_all() implementation for the HyperFeatureRequester is currently reading the entire response (as a test try a payload that is more than 8000 bytes long).

Requirements

Related issues

Provide links to any issues in this repository or elsewhere relating to this pull request.

Describe the solution you've provided

Provide a clear and concise description of what you expect to happen.

Describe alternatives you've considered

Provide a clear and concise description of any alternative solutions or features you've considered.

Additional context

Add any other context about the pull request here.

keelerm84 commented 3 months ago

My apologies for the delay on this PR. There are a few other matters I still have to attend to this week, but I will try to get some resolution on this for you by the end of the week.

Thank you for your patience.

aalexandrov commented 3 months ago

@keelerm84 It's not urgent on our side, it's just something I noticed when I was trying something out.

keelerm84 commented 3 months ago

@aalexandrov Thanks for letting me know this wasn't urgent as it gave me some breathing room. I appreciate that!

I was looking into this change this morning, and I was curious what issue you are trying to solve. You mention in the original write-up that I should:

as a test try a payload that is more than 8000 bytes long

with the default rust SDK version, this seems to work fine for my payload (~12k bytes). Were you experiencing a memory or CPU spike?

aalexandrov commented 2 months ago

I think I was trying to create a mock API endpoint that only implements the regular HTTP protocol (no streaming API) and returns a sizeable JSON response. I wasn't able to make my client SDK consume the entire response without this change.

keelerm84 commented 2 months ago

Hmmm. I can't seem to reproduce this problem against our systems. I am using a payload that is around 17k bytes and it seems to work without issue.

Are you able to provide a reproduction case?

aalexandrov commented 2 months ago

I'm sorry, I think my mock Python server was on a WIP branch that has been lost.

keelerm84 commented 2 months ago

No worries at all. However, without a clear reproduction case, I'm going to close this PR. Please do not hesitate to re-open this issue if you are able to reproduce this error.

Thank you for all of the time and effort you have invested!