Skip to content
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

Python: .Net: ADR for realtime #10355

Open
wants to merge 17 commits into
base: main
Choose a base branch
from

Conversation

eavanvalkenburg
Copy link
Member

Motivation and Context

ADR describing the preliminary design for realtime API's.
As these API's themselves are in preview, we might have to deal with incompatible changes down the road.

Description

Contribution Checklist

@eavanvalkenburg eavanvalkenburg added .NET Issue or Pull requests regarding .NET code python Pull requests for the Python Semantic Kernel labels Feb 10, 2025
@github-actions github-actions bot changed the title ADR for realtime Python: ADR for realtime Feb 10, 2025
@github-actions github-actions bot changed the title Python: ADR for realtime .Net: ADR for realtime Feb 10, 2025
@markwallace-microsoft markwallace-microsoft removed .NET Issue or Pull requests regarding .NET code python Pull requests for the Python Semantic Kernel labels Feb 11, 2025
@eavanvalkenburg eavanvalkenburg added the python Pull requests for the Python Semantic Kernel label Feb 11, 2025
@github-actions github-actions bot changed the title .Net: ADR for realtime Python: .Net: ADR for realtime Feb 11, 2025
@markwallace-microsoft markwallace-microsoft removed the python Pull requests for the Python Semantic Kernel label Feb 11, 2025
@eavanvalkenburg eavanvalkenburg force-pushed the realtime_adr branch 3 times, most recently from afa206e to 30e4b81 Compare February 12, 2025 13:14

It might also be possible that a single event from the service contains multiple content items, for instance a response might contain both text and audio, in that case multiple events will be emitted. It might also be that a single service event is represented twice, i.e. once as a AudioEvent and once as a ServiceEvent, this once again gives the most flexibility to the developer.

```python
Copy link
Member

@RogerBarreto RogerBarreto Feb 12, 2025

Choose a reason for hiding this comment

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

Some observations.

  1. Should we instead of having specialized Image/Audio events, have ContentEvent with the specific content within it?

I'm asking that because in M.E.AI we dropped the Image and Audio specialization in favor of Data for any binary information.

  1. I would suggest that all events should be ServiceEvent specializations. Following also @westey-m point, I would suggest having the inner_event|raw_representation for the base ServiceEvent where this would be the way to get the event name, not fully sure of exposing the underlying service event name as part of the abstraction.

Copy link
Member Author

Choose a reason for hiding this comment

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

I can see the logic in this, however these events are going both ways, you also create them to send data to the service and not all of those have content, some are empty, but you just call with the right service event type, so that isn't always the way to get the event name!

Copy link
Member Author

Choose a reason for hiding this comment

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

and only having a content event and then not having a different internal type for image or audio (as they are both data), means that you would have to dig into the actual event or match on the event type, that seems like a worse experience

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants