feat: allow opt-out of schema dump to dict #57
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
psychedelicious
added a commit
to invoke-ai/InvokeAI
that referenced
this pull request
Feb 19, 2024
Our events handling and implementation is not very friendly: - We have to define functions for every type of event we want to emit and call those specific functions. - Adding or removing data from payloads requires changes wherever the events are dispatched from, and in the events service. - We have no type safety for events and need to rely on string matching and dict access when interacting with events. `fastapi_events` has a neat feature where you can create a pydantic model as an event payload, give it an `__event_name__` attr, and then dispatch the model directly. This allows us to eliminate a layer of indirection and some unpleasant complexity: - We do not need functions for every event type. Define the event in a single model, and dispatch the model directly. The events service only has a single `dispatch` method. - Event handler callbacks get type hints for their event payloads, and can use `isinstance` on them if needed. *see note below* - Event payload construction is now the responsibility of the event itself, not the service. Every event model has a `build` class method, encapsulating this logic. - We can generate OpenAPI schemas for the event payloads and get type safety on the frontend. Previously, the types were manually created (and bugs _have_ occurred when a payload changed on the backend). - When registering event callbacks, we can now register the event model itself instead of its event name. Impossible to make typos. This commit moves the backend over to this improved event handling setup. *Note* Actually, `fastapi_events` has baked in conversion of pydantic events to dicts, so event callbacks get an untyped dict instead of a pydantic model. I've raised a PR to make this behaviour configurable: melvinkcx/fastapi-events#57 I used my `fastapi_events` PR branch while working on this PR, so you'd need to use it to test: https://github.com/psychedelicious/fastapi-events/tree/psyche/feat/payload-schema-dump
psychedelicious
added a commit
to invoke-ai/InvokeAI
that referenced
this pull request
Feb 19, 2024
Our events handling and implementation is not very friendly: - We have to define functions for every type of event we want to emit and call those specific functions. - Adding or removing data from payloads requires changes wherever the events are dispatched from, and in the events service. - We have no type safety for events and need to rely on string matching and dict access when interacting with events. `fastapi_events` has a neat feature where you can create a pydantic model as an event payload, give it an `__event_name__` attr, and then dispatch the model directly. This allows us to eliminate a layer of indirection and some unpleasant complexity: - We do not need functions for every event type. Define the event in a single model, and dispatch the model directly. The events service only has a single `dispatch` method. - Event handler callbacks get type hints for their event payloads, and can use `isinstance` on them if needed. *see note below* - Event payload construction is now the responsibility of the event itself, not the service. Every event model has a `build` class method, encapsulating this logic. - We can generate OpenAPI schemas for the event payloads and get type safety on the frontend. Previously, the types were manually created (and bugs _have_ occurred when a payload changed on the backend). - When registering event callbacks, we can now register the event model itself instead of its event name. Impossible to make typos. This commit moves the backend over to this improved event handling setup. *Note* Actually, `fastapi_events` has baked in conversion of pydantic events to dicts, so event callbacks get an untyped dict instead of a pydantic model. I've raised a PR to make this behaviour configurable: melvinkcx/fastapi-events#57 I used my `fastapi_events` PR branch while working on this PR, so you'd need to use it to test: https://github.com/psychedelicious/fastapi-events/tree/psyche/feat/payload-schema-dump
|
I merged your #55 and it's causing merge conflict now. Please pull from the upstream again. BTW, I also noticed I didn't enable Gitlab Action on Again, thanks for your contribution. |
- Adds `payload_schema_dump: bool = True` arg to `dispatch`, `_derive_event_name_and_payload_from_pydantic_model` and `_validate_payload`. When `False`, the event handlers are called "live" pydantic model for payload. The default behaviour is unchanged. - Updates tests for this new functionality. Closes melvinkcx#56
psychedelicious
force-pushed
the
psyche/feat/payload-schema-dump
branch
from
d5e373f
to
5b43e2f
Compare
|
@melvinkcx Updated. Happy to contribute! Locally, I noticed flake8: OK (1.54=setup[1.42]+cmd[0.12] seconds)
isort: OK (0.96=setup[0.89]+cmd[0.07] seconds)
py37-starlite: SKIP (0.00 seconds)
py38-starlite: SKIP (0.00 seconds)
py39-starlite: SKIP (0.00 seconds)
py310-starlite: OK (2.21=setup[0.98]+cmd[1.23] seconds)
py311-starlite: SKIP (0.01 seconds)
py37-fastapi092-pydantic1: SKIP (0.01 seconds)
py37-fastapi093-pydantic1: SKIP (0.01 seconds)
py37-fastapi094-pydantic1: SKIP (0.01 seconds)
py37-fastapi095-pydantic1: SKIP (0.01 seconds)
py37-fastapi096-pydantic1: SKIP (0.01 seconds)
... # snip
congratulations :) (47.75 seconds) |
psychedelicious
added a commit
to invoke-ai/InvokeAI
that referenced
this pull request
Feb 20, 2024
Our events handling and implementation is not very friendly: - We have to define functions for every type of event we want to emit and call those specific functions. - Adding or removing data from payloads requires changes wherever the events are dispatched from, and in the events service. - We have no type safety for events and need to rely on string matching and dict access when interacting with events. `fastapi_events` has a neat feature where you can create a pydantic model as an event payload, give it an `__event_name__` attr, and then dispatch the model directly. This allows us to eliminate a layer of indirection and some unpleasant complexity: - We do not need functions for every event type. Define the event in a single model, and dispatch the model directly. The events service only has a single `dispatch` method. - Event handler callbacks get type hints for their event payloads, and can use `isinstance` on them if needed. *see note below* - Event payload construction is now the responsibility of the event itself, not the service. Every event model has a `build` class method, encapsulating this logic. - We can generate OpenAPI schemas for the event payloads and get type safety on the frontend. Previously, the types were manually created (and bugs _have_ occurred when a payload changed on the backend). - When registering event callbacks, we can now register the event model itself instead of its event name. Impossible to make typos. This commit moves the backend over to this improved event handling setup. *Note* Actually, `fastapi_events` has baked in conversion of pydantic events to dicts, so event callbacks get an untyped dict instead of a pydantic model. I've raised a PR to make this behaviour configurable: melvinkcx/fastapi-events#57 I used my `fastapi_events` PR branch while working on this PR, so you'd need to use it to test: https://github.com/psychedelicious/fastapi-events/tree/psyche/feat/payload-schema-dump
|
Yeah, tox runs tests based on the environment it's in. If it can't detect the Python version it needs, it will skip them. What you're seeing is normal. |
|
Ah ok. Thanks for explaining how tox works. Nifty. |
psychedelicious
added a commit
to invoke-ai/InvokeAI
that referenced
this pull request
Feb 21, 2024
Our events handling and implementation is not very friendly: - We have to define functions for every type of event we want to emit and call those specific functions. - Adding or removing data from payloads requires changes wherever the events are dispatched from, and in the events service. - We have no type safety for events and need to rely on string matching and dict access when interacting with events. `fastapi_events` has a neat feature where you can create a pydantic model as an event payload, give it an `__event_name__` attr, and then dispatch the model directly. This allows us to eliminate a layer of indirection and some unpleasant complexity: - We do not need functions for every event type. Define the event in a single model, and dispatch the model directly. The events service only has a single `dispatch` method. - Event handler callbacks get type hints for their event payloads, and can use `isinstance` on them if needed. *see note below* - Event payload construction is now the responsibility of the event itself, not the service. Every event model has a `build` class method, encapsulating this logic. - We can generate OpenAPI schemas for the event payloads and get type safety on the frontend. Previously, the types were manually created (and bugs _have_ occurred when a payload changed on the backend). - When registering event callbacks, we can now register the event model itself instead of its event name. Impossible to make typos. This commit moves the backend over to this improved event handling setup. *Note* Actually, `fastapi_events` has baked in conversion of pydantic events to dicts, so event callbacks get an untyped dict instead of a pydantic model. I've raised a PR to make this behaviour configurable: melvinkcx/fastapi-events#57 I used my `fastapi_events` PR branch while working on this PR, so you'd need to use it to test: https://github.com/psychedelicious/fastapi-events/tree/psyche/feat/payload-schema-dump
psychedelicious
added a commit
to invoke-ai/InvokeAI
that referenced
this pull request
Mar 10, 2024
Our events handling and implementation is not very friendly: - We have to define functions for every type of event we want to emit and call those specific functions. - Adding or removing data from payloads requires changes wherever the events are dispatched from, and in the events service. - We have no type safety for events and need to rely on string matching and dict access when interacting with events. `fastapi_events` has a neat feature where you can create a pydantic model as an event payload, give it an `__event_name__` attr, and then dispatch the model directly. This allows us to eliminate a layer of indirection and some unpleasant complexity: - We do not need functions for every event type. Define the event in a single model, and dispatch the model directly. The events service only has a single `dispatch` method. - Event handler callbacks get type hints for their event payloads, and can use `isinstance` on them if needed. *see note below* - Event payload construction is now the responsibility of the event itself, not the service. Every event model has a `build` class method, encapsulating this logic. - We can generate OpenAPI schemas for the event payloads and get type safety on the frontend. Previously, the types were manually created (and bugs _have_ occurred when a payload changed on the backend). - When registering event callbacks, we can now register the event model itself instead of its event name. Impossible to make typos. This commit moves the backend over to this improved event handling setup. *Note* Actually, `fastapi_events` has baked in conversion of pydantic events to dicts, so event callbacks get an untyped dict instead of a pydantic model. I've raised a PR to make this behaviour configurable: melvinkcx/fastapi-events#57 I used my `fastapi_events` PR branch while working on this PR, so you'd need to use it to test: https://github.com/psychedelicious/fastapi-events/tree/psyche/feat/payload-schema-dump
psychedelicious
added a commit
to invoke-ai/InvokeAI
that referenced
this pull request
Mar 10, 2024
Our events handling and implementation is not very friendly: - We have to define functions for every type of event we want to emit and call those specific functions. - Adding or removing data from payloads requires changes wherever the events are dispatched from, and in the events service. - We have no type safety for events and need to rely on string matching and dict access when interacting with events. `fastapi_events` has a neat feature where you can create a pydantic model as an event payload, give it an `__event_name__` attr, and then dispatch the model directly. This allows us to eliminate a layer of indirection and some unpleasant complexity: - We do not need functions for every event type. Define the event in a single model, and dispatch the model directly. The events service only has a single `dispatch` method. - Event handler callbacks get type hints for their event payloads, and can use `isinstance` on them if needed. *see note below* - Event payload construction is now the responsibility of the event itself, not the service. Every event model has a `build` class method, encapsulating this logic. - We can generate OpenAPI schemas for the event payloads and get type safety on the frontend. Previously, the types were manually created (and bugs _have_ occurred when a payload changed on the backend). - When registering event callbacks, we can now register the event model itself instead of its event name. Impossible to make typos. This commit moves the backend over to this improved event handling setup. *Note* Actually, `fastapi_events` has baked in conversion of pydantic events to dicts, so event callbacks get an untyped dict instead of a pydantic model. I've raised a PR to make this behaviour configurable: melvinkcx/fastapi-events#57 I used my `fastapi_events` PR branch while working on this PR, so you'd need to use it to test: https://github.com/psychedelicious/fastapi-events/tree/psyche/feat/payload-schema-dump
psychedelicious
added a commit
to invoke-ai/InvokeAI
that referenced
this pull request
Mar 14, 2024
Our events handling and implementation is not very friendly: - We have to define functions for every type of event we want to emit and call those specific functions. - Adding or removing data from payloads requires changes wherever the events are dispatched from, and in the events service. - We have no type safety for events and need to rely on string matching and dict access when interacting with events. `fastapi_events` has a neat feature where you can create a pydantic model as an event payload, give it an `__event_name__` attr, and then dispatch the model directly. This allows us to eliminate a layer of indirection and some unpleasant complexity: - We do not need functions for every event type. Define the event in a single model, and dispatch the model directly. The events service only has a single `dispatch` method. - Event handler callbacks get type hints for their event payloads, and can use `isinstance` on them if needed. *see note below* - Event payload construction is now the responsibility of the event itself, not the service. Every event model has a `build` class method, encapsulating this logic. - We can generate OpenAPI schemas for the event payloads and get type safety on the frontend. Previously, the types were manually created (and bugs _have_ occurred when a payload changed on the backend). - When registering event callbacks, we can now register the event model itself instead of its event name. Impossible to make typos. This commit moves the backend over to this improved event handling setup. *Note* Actually, `fastapi_events` has baked in conversion of pydantic events to dicts, so event callbacks get an untyped dict instead of a pydantic model. I've raised a PR to make this behaviour configurable: melvinkcx/fastapi-events#57 I used my `fastapi_events` PR branch while working on this PR, so you'd need to use it to test: https://github.com/psychedelicious/fastapi-events/tree/psyche/feat/payload-schema-dump chore(ui): typegen refactor(ui): update frontend to use new events setup TODO: Support session_started and session_canceled events. feat(events): improved types build: pin fastapi-events to unreleased version It's merged, but not released yet. fix(events): fix merge issue chore(ui): lint feat(events): migrate MM install events to pydantic feat(ui): update events for new mm install chore: bump fastapi_events feat(events): revise how events are dispatched Dispatching the fully-formed event payloads directly ended up causing circular import issues. Revised so that each event has its own `emit_...` method on the events service, which is how it was originally. This also makes it clear what events are valid in the system.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
payload_schema_dump: bool = Truearg todispatch,_derive_event_name_and_payload_from_pydantic_modeland_validate_payload. WhenFalse, the event handlers are called "live" pydantic model for payload. The default behaviour is unchanged.Closes #56