Skip to content

Conversation

tconley1428
Copy link
Contributor

@tconley1428 tconley1428 commented Jul 30, 2025

What was changed

Added ability to configure replayer to plugins. Plugins as a whole have become abstract so that plugin authors need to consider the different components they may need to configure.

Why?

Checklist

  1. Closes [Feature Request] Worker plugin support for replayers #994

  2. How was this tested:

  1. Any docs updates needed?

@tconley1428 tconley1428 changed the title POC for replayer configuration from existing plugins Replayer configuration from plugins Aug 4, 2025
@tconley1428 tconley1428 marked this pull request as ready for review August 4, 2025 21:47
@tconley1428 tconley1428 requested a review from a team as a code owner August 4, 2025 21:47
Copy link
Member

@Sushisource Sushisource left a comment

Choose a reason for hiding this comment

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

Looking good to me. If we could, I would blow everything up and extract the things that are needed for deterministic replay out of the client/worker config so there's only one thing, but, that ship's long sailed.

Copy link
Member

@cretz cretz left a comment

Choose a reason for hiding this comment

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

Looks great. Also, even though they were experimental anyways, we like to call out breaking changes in the release notes, so lets try to remember to do that.

@tconley1428 tconley1428 changed the title Replayer configuration from plugins 💥 Replayer configuration from plugins Aug 6, 2025
@tconley1428 tconley1428 merged commit 71ed93c into main Aug 7, 2025
45 of 52 checks passed
@tconley1428 tconley1428 deleted the replayer_plugin branch August 7, 2025 00:04

def configure_replayer(self, config: ReplayerConfig) -> ReplayerConfig:
"""Configure the replayer for OpenAI Agents."""
config["interceptors"] = list(config.get("interceptors") or []) + [

Choose a reason for hiding this comment

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

This seems like a very surprising footgun and introduces coupling, and requires that users understand a concept (Replayer) that few are familiar with. It feels like a very large chance that users will mess this up.

Copy link
Member

Choose a reason for hiding this comment

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

This is why we chose to require that plugin authors configure replayers and therefore become familiar with them. Regular users don't need to become familiar with replayers, but plugin authors do to ensure their plugin works right in replayer scenarios and not just client and worker scenarios.

config["data_converter"] = pydantic_data_converter
return config

async def run_replayer(
Copy link

@DouweM DouweM Aug 27, 2025

Choose a reason for hiding this comment

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

@tconley1428 This async shouldn't be here


def configure_replayer(self, config: ReplayerConfig) -> ReplayerConfig:
config["data_converter"] = pydantic_data_converter
return config
Copy link

Choose a reason for hiding this comment

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

@tconley1428 This should call self.next_worker_plugin.configure_replayer right?

tconley1428 added a commit that referenced this pull request Aug 28, 2025
* POC for replayer configuration from existing plugins

* Handle non-combined cases

* Fixing type checking

* Move shared configuration into plugin definition

* Moving replay configuration to worker plugin, add replay execution hook

* Readme and some cleanup

* Remove debug logging

* Make plugin interface experimental and abstract

* Update readme

* Update readme

* Readme fixes, move plugin to its own file

* Fix readme

* Update temporalio/worker/_plugin.py

Co-authored-by: Spencer Judge <[email protected]>

* Rename run_replayer, remove need to return self from init

* Docstring change

---------

Co-authored-by: Spencer Judge <[email protected]>
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.

[Feature Request] Worker plugin support for replayers

5 participants