Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add support for defining streams #4513

Merged
merged 33 commits into from
Oct 7, 2024
Merged

Conversation

chrisradek
Copy link
Member

Related: #154

@azure-sdk
Copy link
Collaborator

azure-sdk commented Sep 24, 2024

All changed packages have been documented.

  • @typespec/compiler
  • @typespec/events
  • @typespec/http
  • @typespec/sse
  • @typespec/streams
Show changes

@typespec/events - feature ✏️

Adds a new core package for describing events.

@typespec/streams - feature ✏️

Adds a new core package for describing streams and the type of data they contain.

@typespec/sse - feature ✏️

Adds a new core package to describe server-sent events.

@typespec/http - feature ✏️

Adds HttpStream and JsonlStream models to to support streaming use-cases.

@typespec/compiler - fix ✏️

Fixes issue with the semantic walker where exitTuple was not being emitted.

@azure-sdk
Copy link
Collaborator

You can try these changes here

🛝 Playground 🌐 Website 📚 Next docs

@chrisradek chrisradek marked this pull request as ready for review September 25, 2024 22:41
@chrisradek chrisradek changed the title [WIP] Add support for defining streams Add support for defining streams Sep 25, 2024
@chrisradek
Copy link
Member Author

Azure/typespec-azure#1615 addresses the e2e test failures.

Copy link
Member

@timotheeguerin timotheeguerin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think just need to update the init templates to include @stream and maybe bring this up tomorrow in scrum to make sure we want to add this new dependency on http

@chrisradek chrisradek added this pull request to the merge queue Oct 7, 2024
Merged via the queue into microsoft:main with commit f69f8fa Oct 7, 2024
22 checks passed
@chrisradek chrisradek deleted the add-streaming branch October 7, 2024 23:43
@@ -0,0 +1,42 @@
---
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this docs/libraries/stream/reference duplicate with docs/libraries/streams/reference?

@qiaozha
Copy link
Member

qiaozha commented Oct 22, 2024

Seems like the playground is not working here link I wonder if we should add those libraries into the playground?

Also, the openapi3 output from my local is generating the response type as string,

    get:
      operationId: subscribe
      parameters: []
      responses:
        '200':
          description: The request has succeeded.
          content:
            application/jsonl:
              schema:
                type: string

is this expected?

If yes, how can we link the output type to the model Events? is there some apis for it, I notice this is empty for now https://github.com/microsoft/typespec/pull/4513/files#diff-dd2a7fbf51ff9caf7d341ebb695101d0f48e761bc9e4a2df96411bcf3ee33647R2

* }
*
* @TypeSpec.Events.events
* union ChannelEvents {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just curious if we should suggest TypeSpec author to use discriminated union if there's multiple event types? I think client side probably need to handle them differently?

* usermessage: UserMessage,
* userdisconnect: UserDisconnect,
*
* @Events.contentType("text/plain")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess different event could have different content type? I wonder how can we get the context type for a specific event type?

* Message,
* }
*
* op subscribe(): JsonlStream<Events>;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am a little confused by the jsonlstream usage and the event usage, does jsonl stream necessarily mean it's an event based?

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants