-
Notifications
You must be signed in to change notification settings - Fork 12
Add ingress client #282
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
Add ingress client #282
Conversation
c7dac5b
to
fb782b3
Compare
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.
Left a couple of comments.
|
||
export interface IngresCallOptions { | ||
idempotencyKey?: string; | ||
retain?: number; |
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.
Can you allow to add additional headers?
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.
ah!, good idea!
idempotencyKey?: string; | ||
retain?: number; |
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.
A thing I would have done differently is to group the idempotency options all together, like
export interface IdempotencyOptions {
key: string;
retain?: number;
}
export interface IngressCallOptions {
idempotency?: IdempotencyOptions;
headers: {[string]: string};
}
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.
I'd rather to keep it flat for now.
fragments.push(params.handler); | ||
if (params.send ?? false) { | ||
if (params.delay) { | ||
fragments.push(`send?delay=${params.delay}`); |
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.
This is now number right?
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.
This is indeed a number, like the retention number. We said that we would like to align both of them once that feature will be implemented.
This commit adds the first version of a typed client. See examples/ingress.ts for a usage example.
3b41e6c
to
9ed4ae2
Compare
This PR adds the first version of a typed client, that can be used outside of restate to communicate with restate backed services/objects .
See examples/ingress.ts for a usage example.