launchdarkly / ld-find-code-refs

Build tool for automatically sending feature flag code references to LaunchDarkly
https://launchdarkly.com
Other
46 stars 34 forks source link

fix: use RateLimitBackoff client in LD API #437

Closed jonathan-ostrander closed 6 months ago

jonathan-ostrander commented 6 months ago

https://github.com/launchdarkly/ld-find-code-refs/pull/434 attempted to handle the rate limit from the LD API but the LD API client uses http.DefaultClient by default. This change passes in the client with RateLimitBackoff set into the call to init the LD API client.

jonathan-ostrander commented 6 months ago

@jazanne sorry for needing a follow-up PR, but the original PR did not actually affect any of the LD API calls. This should have the intended behavior with regards to the API rate limits.

jazanne commented 6 months ago

@jonathan-ostrander thanks for raising this issue. We've decided to remove the use of that secondary api client here. Hope to have a release out by end of week. Thank you for your patience

jonathan-ostrander commented 6 months ago

@jazanne could we consider this a hot fix for the current build while that PR gets through review? I'm worried that such a large PR might take longer than the next couple of days to be merged.

jazanne commented 6 months ago

@jonathan-ostrander it looks like tests are failing with nil pointer deference related to the log line you added

Failed
panic: runtime error: invalid memory address or nil pointer dereference
/usr/local/go/src/testing/testing.go:1526 +0x372
/usr/local/go/src/testing/testing.go:1529 +0x650
/usr/local/go/src/runtime/panic.go:890 +0x263
/home/circleci/project/internal/ld/ld.go:88 +0x2dc
/home/circleci/project/internal/ld/ld_test.go:272 +0x2d2

If we get a fix for that I can try getting a hotfix out today

jonathan-ostrander commented 6 months ago

Whoops, forgot to add the Request to the test fixture. Fixed in ff79d49.