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: Event Streaming #2482

Merged
merged 27 commits into from
Apr 11, 2023
Merged

Python: Event Streaming #2482

merged 27 commits into from
Apr 11, 2023

Conversation

unexge
Copy link
Contributor

@unexge unexge commented Mar 21, 2023

Motivation and Context

Closes #1752

Description

Testing

TODOs

  • Update PyO3 and PyO3Asyncio to latest

Checklist

  • I have updated CHANGELOG.next.toml if I made changes to the smithy-rs codegen or runtime crates
  • I have updated CHANGELOG.next.toml if I made changes to the AWS SDK, generated SDK code, or SDK runtime crates

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@github-actions
Copy link

Copy link
Contributor

@crisidev crisidev left a comment

Choose a reason for hiding this comment

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

Somehow the codegen diff is showning this PR is reverting to an older version of tokio that is affected by a CVE and @rcoh fixed a couple of days ago: #2474

@unexge unexge force-pushed the unexge/python-event-streaming branch from 7498599 to b610cb2 Compare March 28, 2023 08:53
@github-actions
Copy link

A new generated diff is ready to view.

A new doc preview is ready to view.

@github-actions
Copy link

A new generated diff is ready to view.

A new doc preview is ready to view.

@unexge unexge force-pushed the unexge/python-event-streaming branch 2 times, most recently from 21209b9 to b7afb23 Compare March 30, 2023 17:18
@github-actions
Copy link

A new generated diff is ready to view.

  • AWS SDK (ignoring whitespace)
  • No codegen difference in the Client Test
  • No codegen difference in the Server Test
  • No codegen difference in the Server Test Python

A new doc preview is ready to view.

@unexge unexge force-pushed the unexge/python-event-streaming branch from b7afb23 to 86b681f Compare March 31, 2023 10:50
@github-actions
Copy link

A new generated diff is ready to view.

  • AWS SDK (ignoring whitespace)
  • No codegen difference in the Client Test
  • No codegen difference in the Server Test
  • No codegen difference in the Server Test Python

A new doc preview is ready to view.

Copy link
Contributor Author

@unexge unexge left a comment

Choose a reason for hiding this comment

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

Reviewed with @crisidev

codegen-server-test/python/model/pokemon.smithy Outdated Show resolved Hide resolved
}

companion object {
data class EventStreamSymbol(val innerT: RustType, val errorT: RustType)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let's discuss about this approach

private val pyO3Asyncio = PythonServerCargoDependency.PyO3Asyncio.toType()
private val tokio = PythonServerCargoDependency.Tokio.toType()
private val futures = PythonServerCargoDependency.Futures.toType()
private val parkingLot = PythonServerCargoDependency.ParkingLot.toType()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let's create an issue to whether we can switch to std::sync::Mutex instead of parking_lot::Mutex

@unexge unexge force-pushed the unexge/python-event-streaming branch from 86b681f to c8ee180 Compare April 10, 2023 12:42
@unexge unexge marked this pull request as ready for review April 10, 2023 12:43
@unexge unexge requested review from a team as code owners April 10, 2023 12:43
@github-actions
Copy link

A new generated diff is ready to view.

A new doc preview is ready to view.

Copy link
Contributor

@crisidev crisidev left a comment

Choose a reason for hiding this comment

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

LGTM!

@crisidev crisidev enabled auto-merge April 11, 2023 11:04
@github-actions
Copy link

A new generated diff is ready to view.

A new doc preview is ready to view.

@crisidev crisidev added this pull request to the merge queue Apr 11, 2023
Merged via the queue into main with commit 1a7a495 Apr 11, 2023
@crisidev crisidev deleted the unexge/python-event-streaming branch April 11, 2023 11:50
Velfi pushed a commit that referenced this pull request Apr 17, 2023
* Add `RustType.Application` and `PythonType.Application` variants

* Use `RustType.Application` for event streaming symbols

* Call symbol provider to get `Receiver` type instead of hardcoding

* Add Python specific event streaming symbol provider

* Add event streaming wrapper generator

* Generate correct type information for event streaming types

* Add `CapturePokemon` operation to Python Pokemon service

* Add `InternalServerError` variant to all event streaming errors

* Use `PythonServerCargoDependency` for PyO3Asyncio imports

* Return an attribute error instead of panicking in `IntoPy` impls of wrappers

* Add `Sync` bound to `new` methods of wrappers

* Revert "Add `InternalServerError` variant to all event streaming errors"

This reverts commit b610cb2.

* Add `PythonServerEventStreamErrorGenerator` to generate Python specific error types for unions

* Try to extract error type or inner type from incoming streaming value and ignore the value otherwise for now

* Allow missing type-stubs for `aiohttp`

* Propogate modelled errors through stream

* Raise modelled exceptions rather than sending through stream

* Allow senders from Python side to raise modelled exceptions

* Update `EventStreamSymbolProviderTest` to use `Application` type instead of `Opaque` type

* Fix `ktlint` issues

* Group up common variables in `codegenScope`

* Document `RustType.Application`

* Format `codegen-server-test/python/model/pokemon.smithy`

* Use `tokio-stream` crate instead of `futures` crate

* Use a better error message if user tries to reuse a stream

* Add some details about event streams to example Pokemon service

---------

Co-authored-by: Matteo Bigoi <1781140+crisidev@users.noreply.github.com>
unexge added a commit that referenced this pull request Apr 24, 2023
* Add `RustType.Application` and `PythonType.Application` variants

* Use `RustType.Application` for event streaming symbols

* Call symbol provider to get `Receiver` type instead of hardcoding

* Add Python specific event streaming symbol provider

* Add event streaming wrapper generator

* Generate correct type information for event streaming types

* Add `CapturePokemon` operation to Python Pokemon service

* Add `InternalServerError` variant to all event streaming errors

* Use `PythonServerCargoDependency` for PyO3Asyncio imports

* Return an attribute error instead of panicking in `IntoPy` impls of wrappers

* Add `Sync` bound to `new` methods of wrappers

* Revert "Add `InternalServerError` variant to all event streaming errors"

This reverts commit b610cb2.

* Add `PythonServerEventStreamErrorGenerator` to generate Python specific error types for unions

* Try to extract error type or inner type from incoming streaming value and ignore the value otherwise for now

* Allow missing type-stubs for `aiohttp`

* Propogate modelled errors through stream

* Raise modelled exceptions rather than sending through stream

* Allow senders from Python side to raise modelled exceptions

* Update `EventStreamSymbolProviderTest` to use `Application` type instead of `Opaque` type

* Fix `ktlint` issues

* Group up common variables in `codegenScope`

* Document `RustType.Application`

* Format `codegen-server-test/python/model/pokemon.smithy`

* Use `tokio-stream` crate instead of `futures` crate

* Use a better error message if user tries to reuse a stream

* Add some details about event streams to example Pokemon service

---------

Co-authored-by: Matteo Bigoi <1781140+crisidev@users.noreply.github.com>
rcoh pushed a commit that referenced this pull request Apr 24, 2023
* Add `RustType.Application` and `PythonType.Application` variants

* Use `RustType.Application` for event streaming symbols

* Call symbol provider to get `Receiver` type instead of hardcoding

* Add Python specific event streaming symbol provider

* Add event streaming wrapper generator

* Generate correct type information for event streaming types

* Add `CapturePokemon` operation to Python Pokemon service

* Add `InternalServerError` variant to all event streaming errors

* Use `PythonServerCargoDependency` for PyO3Asyncio imports

* Return an attribute error instead of panicking in `IntoPy` impls of wrappers

* Add `Sync` bound to `new` methods of wrappers

* Revert "Add `InternalServerError` variant to all event streaming errors"

This reverts commit b610cb2.

* Add `PythonServerEventStreamErrorGenerator` to generate Python specific error types for unions

* Try to extract error type or inner type from incoming streaming value and ignore the value otherwise for now

* Allow missing type-stubs for `aiohttp`

* Propogate modelled errors through stream

* Raise modelled exceptions rather than sending through stream

* Allow senders from Python side to raise modelled exceptions

* Update `EventStreamSymbolProviderTest` to use `Application` type instead of `Opaque` type

* Fix `ktlint` issues

* Group up common variables in `codegenScope`

* Document `RustType.Application`

* Format `codegen-server-test/python/model/pokemon.smithy`

* Use `tokio-stream` crate instead of `futures` crate

* Use a better error message if user tries to reuse a stream

* Add some details about event streams to example Pokemon service

---------

Co-authored-by: Matteo Bigoi <1781140+crisidev@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
python-server Python server SDK
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Python] Support event streaming trait
3 participants