chore: remove GRPC request and migrate to use REST API#186
chore: remove GRPC request and migrate to use REST API#186duyhungtnn wants to merge 41 commits intomasterfrom
Conversation
duyhungtnn
changed the title
There was a problem hiding this comment.
Pull request overview
This PR migrates the Node SDK from gRPC to REST API for feature flag and segment user retrieval. The migration removes gRPC dependencies, implements a REST-based APIClient with JSON payloads, and adds converters to transform REST responses into protobuf objects required by the local evaluator.
Changes:
- Removed gRPC client implementation and dependencies (
@improbable-eng/grpc-web,@improbable-eng/grpc-web-node-http-transport) - Added REST API endpoints for
getFeatureFlagsandgetSegmentUsersinAPIClient - Implemented converter functions to transform plain TypeScript objects from REST responses into protobuf objects
- Updated cache processors to use
APIClientinstead ofGRPCClient - Updated all related tests and mocks to work with REST responses
Reviewed changes
Copilot reviewed 31 out of 33 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
src/objects/segment.ts |
New type definitions for segment users (REST) |
src/objects/feature.ts |
New type definitions for features (REST) |
src/objects/request.ts |
Added request types for new REST endpoints |
src/objects/response.ts |
Added response types for new REST endpoints |
src/evaluator/converter.ts |
New converter logic to transform REST objects to protobuf |
src/api/client.ts |
Added getFeatureFlags and getSegmentUsers REST endpoints |
src/cache/processor/*.ts |
Migrated from GRPCClient to APIClient |
src/index.ts |
Removed gRPC client initialization |
src/client.ts |
Updated comments (gRPC → REST) |
src/grpc/*.ts |
Deleted gRPC implementation files |
src/__tests__/mocks/api.ts |
New mock for APIClient |
src/__tests__/mocks/gprc.ts |
Deleted gRPC mock |
src/__tests__/**/*.ts |
Updated all tests to use REST mocks and responses |
package.json |
Removed gRPC dependencies |
yarn.lock |
Updated dependencies, changed registry URLs from yarnpkg.com to npmjs.org |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 29 out of 31 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 29 out of 31 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 29 out of 31 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
… to Protobuf objects.
…createSegmentUsers` to `toProtoFeature` and `toProtoSegmentUsers` for clarity.
…ock async rejections in feature cache tests.
…ted tests and dependencies.
Delete two planning/analysis docs: evaluation_model_analytics.md and grpc_to_rest_migration_analytics.md. These files contained design notes for the evaluation model converter and the gRPC→REST migration for cache processors and have been removed (obsolete or relocated).
…clude `TTL` in mock cache `put` method arguments.
Delete the packageManager entry (yarn@1.22.22) from package.json to avoid pinning a specific package manager/version in the repo. This reduces tooling enforcement for contributors using different package managers and simplifies the manifest.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 31 out of 34 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…Protobuf format and update local evaluator to use it.
… Bucketeer proto objects within test files and update e2e configuration.
…input parameters.
Add an early return in src/api/client.ts when the response status is not 200 to avoid setting up response handlers and attempting to parse the response body for non-success responses. This prevents unnecessary work and potential errors from processing error responses.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 35 out of 38 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Delete unused imports (Strategy, Clause, createPrerequisite) from src/__tests__/client_local_evaluation.ts and src/__tests__/evaluator/evaluator.ts to clean up imports and silence linter/TS warnings.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 34 out of 37 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Replace shorthand property syntax with explicit key: value pairs in options and t.context objects within src/__tests__/cache/processor/featureCache/init.ts and update.ts to improve readability. No functional changes.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 34 out of 36 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@cre8ivejp please help me to take a look |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 34 out of 36 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This PR migrates the Node SDK from gRPC to REST API for feature flag and segment user retrieval. The migration removes gRPC dependencies, implements a REST-based APIClient with JSON payloads, and adds converters to transform REST responses into protobuf objects required by the local evaluator.
Changes:
Related to bucketeer-io/bucketeer#2431