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

[feat] Add send event endpoints to api server #412

Merged
merged 14 commits into from
Dec 23, 2024
Merged

Conversation

nerdai
Copy link
Contributor

@nerdai nerdai commented Dec 18, 2024

This PR adds a new endpoint in api server allowing users to post a new Event that gets sent to a running workflow of a specified deployment.

I've also added the send_event() method to the Task apiserver/model class.

@coveralls
Copy link

coveralls commented Dec 18, 2024

Coverage Status

coverage: 73.821% (-0.03%) from 73.849%
when pulling 849c631 on nerdai/api-server-send-event
into 9a14319 on main.

@@ -90,7 +90,7 @@ async def test_session_collection_list(client: Any) -> None:
@pytest.mark.asyncio
async def test_task_results(client: Any) -> None:
res = TaskResult(task_id="a_result", history=[], result="some_text", data={})
client.request.return_value = mock.MagicMock(json=lambda: res.model_dump_json())
client.request.return_value = mock.MagicMock(json=lambda: res.model_dump())
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think we had this incorrectly mocked on the basis that httpx.Response.json() returns a dict rather than a str.

I ran into this error when running the new hitl e2e test and the TaskResult was failing Pydantic's validation. The modification made in this PR resolves it.

(The naming on Pydantic is confusing... model_dump works with dict and model_dump_json works with str.)

@nerdai nerdai requested a review from masci December 18, 2024 18:46
@nerdai nerdai marked this pull request as ready for review December 18, 2024 18:46
@@ -109,6 +109,18 @@ async def send_event(self, service_name: str, task_id: str, ev: Event) -> None:
url = f"{self.client.control_plane_url}/sessions/{self.id}/tasks/{task_id}/send_event"
await self.client.request("POST", url, json=event_def.model_dump())

async def send_event_def(self, task_id: str, ev_def: EventDefinition) -> None:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

So, I needed to add this in order for the apiserver to be able to just forward along an EventDefinition to the WorkflowService

@nerdai nerdai changed the title [feat] Add send human response event endpoints to api server [feat] Add send event endpoints to api server Dec 20, 2024
Copy link
Member

@masci masci left a comment

Choose a reason for hiding this comment

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

Thanks for the extra comments, LGTM!

@masci masci merged commit fe36b3b into main Dec 23, 2024
10 checks passed
@masci masci deleted the nerdai/api-server-send-event branch December 23, 2024 08:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add API Server endpoint to be able to send event to a WorkflowService using SessionClient.send_event
4 participants