-
Notifications
You must be signed in to change notification settings - Fork 564
feat(api): Add watch_api.ts with async iterator support and Configuration pattern
#2738
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Rezrazi The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Welcome @Rezrazi! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This pull request introduces a new WatchApi class that provides an async iterator interface for consuming Kubernetes watch events, designed to integrate with the existing API client infrastructure while supporting both streaming and text-based HTTP responses.
- Adds
WatchApiclass with async iterator support for watching Kubernetes resources - Extends
ResponseBodyinterface to support optional streaming viastream()method - Includes comprehensive test suite covering all watch event types, error conditions, and streaming modes
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| src/watch_api.ts | New WatchApi class implementing async iterator pattern for Kubernetes watch with support for streaming and text fallback modes |
| src/watch_api_test.ts | Comprehensive test suite covering event iteration, error handling, query parameters, custom HTTP libraries, and type safety |
| src/index.ts | Exports WatchApi to make it available as part of the public API |
| src/gen/http/http.ts | Extends ResponseBody interface with optional stream() method to enable streaming support |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const timeoutSignal = AbortSignal.timeout(this.requestTimeoutMs); | ||
|
|
||
| requestContext.setSignal(AbortSignal.any([controller.signal, timeoutSignal])); | ||
|
|
||
| try { | ||
| const response = await this.configuration.httpApi.send(requestContext).toPromise(); | ||
|
|
Copilot
AI
Jan 8, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The timeout signal is applied to the entire watch operation including the streaming phase, which will cause the connection to abort after 30 seconds (the default timeout) even if data is actively being received. For long-lived watch connections that remain open for extended periods, this timeout should only apply to the initial connection establishment, not the entire stream duration. Consider using a pattern similar to other streaming implementations where the timeout is only applied until the first response is received, then cleared or replaced with an idle timeout.
| const timeoutSignal = AbortSignal.timeout(this.requestTimeoutMs); | |
| requestContext.setSignal(AbortSignal.any([controller.signal, timeoutSignal])); | |
| try { | |
| const response = await this.configuration.httpApi.send(requestContext).toPromise(); | |
| // Apply the timeout only to the initial request establishment, not the entire stream. | |
| requestContext.setSignal(controller.signal); | |
| const connectionTimeoutId = setTimeout(() => { | |
| controller.abort(); | |
| }, this.requestTimeoutMs); | |
| try { | |
| const response = await this.configuration.httpApi.send(requestContext).toPromise(); | |
| clearTimeout(connectionTimeoutId); |
| await rejects( | ||
| async () => { | ||
| // eslint-disable-next-line @typescript-eslint/no-unused-vars | ||
| for await (const event of watchApi.watch('/api/v1/namespaces/default/pods')) { |
Copilot
AI
Jan 8, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unused variable event.
| for await (const event of watchApi.watch('/api/v1/namespaces/default/pods')) { | |
| for await (const _event of watchApi.watch('/api/v1/namespaces/default/pods')) { |
| const watchApi = new WatchApi(config); | ||
|
|
||
| // eslint-disable-next-line @typescript-eslint/no-unused-vars | ||
| for await (const event of watchApi.watch('/api/v1/namespaces/default/pods', { |
Copilot
AI
Jan 8, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unused variable event.
| async () => { | ||
| // eslint-disable-next-line @typescript-eslint/no-unused-vars | ||
| for await (const event of watchApi.watch('/api/v1/namespaces/default/pods')) { | ||
| // Should not reach here |
Copilot
AI
Jan 8, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For loop variable event is not used in the loop body.
| // Should not reach here | |
| throw new Error('Should not reach here, received event: ' + JSON.stringify(event)); |
| const watchApi = new WatchApi(config); | ||
|
|
||
| // eslint-disable-next-line @typescript-eslint/no-unused-vars | ||
| for await (const event of watchApi.watch('/api/v1/namespaces/default/pods', { |
Copilot
AI
Jan 8, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For loop variable event is not used in the loop body.
This pull request introduces a new
WatchApiclass for watching Kubernetes resources using async iterators, along with comprehensive tests and supporting changes. The new implementation is designed to integrate seamlessly with the existing API client infrastructure, supporting both streaming and text-based HTTP responses, and is fully type-safe. Current implementation was limited as it relied on a direct call to node-fetch, closing the opportunity to usewrapHttpLibrary.The most important changes are:
New Watch API Implementation:
Added a new
WatchApiclass insrc/watch_api.tsthat provides an async iterator interface for consuming Kubernetes watch events. It supports both streaming and text fallback modes, allows passing arbitrary query parameters, and is compatible with custom HTTP libraries. This class also introduces theWatchEventandWatchEventTypetypes for strong typing of events.Updated the
ResponseBodyinterface insrc/gen/http/http.tsto optionally include astream()method, enabling proper support for streaming watch responses.Exports and API Surface:
WatchApifromsrc/index.tsto make it available as part of the public API.Testing:
src/watch_api_test.tscovering construction, event iteration (both streaming and text), handling of all event types (ADDED,MODIFIED,DELETED,BOOKMARK,ERROR), error handling, query parameter passing, empty responses, custom HTTP libraries, and type safety.Examples
Basic example
Custom httpapi (using ky as an http client)