Title: fix: Use context.Background() in client constructors by samuelkim7 · Pull Request #5897 · feast-dev/feast · GitHub
Open Graph Title: fix: Use context.Background() in client constructors by samuelkim7 · Pull Request #5897 · feast-dev/feast
X Title: fix: Use context.Background() in client constructors by samuelkim7 · Pull Request #5897 · feast-dev/feast
Description: What this PR does / why we need it: This PR fixes a critical context canceled bug in the GCS/S3 RegistryStore and DynamoDB OnlineStore constructors. The Bug: Immediate Context Cancellation During testing with the Go feature server (running in a local Docker container) connected to live cloud infrastructure (GCS, Redis), the following error occurred immediately upon startup: {"level":"error","error":"Get \"https://storage.googleapis.com/.../feature_registry.db\": Post \"https://oauth2.googleapis.com/token\": context canceled", "message":"Registry refresh Failed"} Root Cause In the previous implementation, the constructor (NewGCSRegistryStore) initialized cloud clients using context.WithTimeout() and defer cancel(). Because cancel() is called immediately when the constructor returns, the base context for the GCS client becomes invalidated. This leads to an immediate failure when the client attempts to perform its first operation (like fetching an OAuth token or reading the registry), even if the 5-second timeout hasn't elapsed. The Fix: Lifecycle Alignment A storage client's lifecycle should be tied to the server's uptime. Constructor (Long-lived): Changed to use context.Background(). This ensures the client's underlying connection and token-refresh routines remain valid as long as the server is running. Methods (Request-scoped): Kept the 5-second context.WithTimeout() inside individual methods (GetRegistryProto, Teardown). This correctly applies a deadline to specific I/O operations without killing the persistent client. This follows Go's recommended practice of not storing or canceling request-scoped contexts in long-lived objects. Consistency To ensure architectural consistency across the project and prevent similar latent bugs, this fix has been applied to: GCS Registry Store (NewGCSRegistryStore) S3 Registry Store (NewS3RegistryStore) DynamoDB Online Store (NewDynamodbOnlineStore) Which issue(s) this PR fixes: Fixes the context canceled error that prevents the Go feature server from properly connecting to live cloud registries and online stores. Misc Refactored internal variable names (changing lr to rs in NewS3RegistryStore) to align with the naming conventions used in gcs.go and to accurately reflect their purpose as RegistryStore.
Open Graph Description: What this PR does / why we need it: This PR fixes a critical context canceled bug in the GCS/S3 RegistryStore and DynamoDB OnlineStore constructors. The Bug: Immediate Context Cancellation During ...
X Description: What this PR does / why we need it: This PR fixes a critical context canceled bug in the GCS/S3 RegistryStore and DynamoDB OnlineStore constructors. The Bug: Immediate Context Cancellation During ...
Opengraph URL: https://github.com/feast-dev/feast/pull/5897
X: @github
Domain: github.com
| route-pattern | /:user_id/:repository/pull/:id/files(.:format) |
| route-controller | pull_requests |
| route-action | files |
| fetch-nonce | v2:59c27d07-4b1a-ef3e-a400-84126541a573 |
| current-catalog-service-hash | ae870bc5e265a340912cde392f23dad3671a0a881730ffdadd82f2f57d81641b |
| request-id | 9CC0:321DA7:570FF:74797:6A4EA6CA |
| html-safe-nonce | 8cdc7f5ab2296c24b7dc3282621972c94322035590a46a51fa3d9f9429c91f1b |
| visitor-payload | eyJyZWZlcnJlciI6IiIsInJlcXVlc3RfaWQiOiI5Q0MwOjMyMURBNzo1NzBGRjo3NDc5Nzo2QTRFQTZDQSIsInZpc2l0b3JfaWQiOiI4NzQ4NjM1MDk5OTU0NTg3MzM4IiwicmVnaW9uX2VkZ2UiOiJpYWQiLCJyZWdpb25fcmVuZGVyIjoiaWFkIn0= |
| visitor-hmac | 14e49569b29dcfa047231bddccff6463253fa20df16a87e3c2f3a489bc5af4ec |
| hovercard-subject-tag | pull_request:3203775196 |
| github-keyboard-shortcuts | repository,pull-request-list,pull-request-conversation,pull-request-files-changed,copilot |
| google-site-verification | Apib7-x98H0j5cPqHWwSMm6dNU4GmODRoqxLiDzdx9I |
| octolytics-url | https://collector.github.com/github/collect |
| analytics-location | / |
| fb:app_id | 1401488693436528 |
| apple-itunes-app | app-id=1477376905, app-argument=https://github.com/feast-dev/feast/pull/5897/files |
| twitter:image | https://avatars.githubusercontent.com/u/65876994?s=400&v=4 |
| twitter:card | summary_large_image |
| og:image | https://avatars.githubusercontent.com/u/65876994?s=400&v=4 |
| og:image:alt | What this PR does / why we need it: This PR fixes a critical context canceled bug in the GCS/S3 RegistryStore and DynamoDB OnlineStore constructors. The Bug: Immediate Context Cancellation During ... |
| og:site_name | GitHub |
| og:type | object |
| hostname | github.com |
| expected-hostname | github.com |
| None | 41b6ab3ba6d20a71766ac245b5a4a94c6fc672a9cd4da7d44c1b33ab8bf6a21c |
| turbo-cache-control | no-preview |
| diff-view | unified |
| go-import | github.com/feast-dev/feast git https://github.com/feast-dev/feast.git |
| octolytics-dimension-user_id | 57027613 |
| octolytics-dimension-user_login | feast-dev |
| octolytics-dimension-repository_id | 161133770 |
| octolytics-dimension-repository_nwo | feast-dev/feast |
| octolytics-dimension-repository_public | true |
| octolytics-dimension-repository_is_fork | false |
| octolytics-dimension-repository_network_root_id | 161133770 |
| octolytics-dimension-repository_network_root_nwo | feast-dev/feast |
| turbo-body-classes | logged-out env-production page-responsive full-width |
| disable-turbo | true |
| browser-stats-url | https://api.github.com/_private/browser/stats |
| browser-errors-url | https://api.github.com/_private/browser/errors |
| release | e6a744804e8e70f97b4d5a18a94dcc63db22f97a |
| ui-target | canary-1 |
| theme-color | #1e2327 |
| color-scheme | light dark |
Links:
Viewport: width=device-width