Skip to content

Add documentation of interceptors to README #970

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

Merged
merged 3 commits into from
Jul 15, 2025

Conversation

dandavison
Copy link
Contributor

@dandavison dandavison requested a review from a team as a code owner July 15, 2025 02:44
@dandavison dandavison force-pushed the document-client-worker-interceptors branch 2 times, most recently from 400c5a9 to 64121d6 Compare July 15, 2025 02:59
@dandavison dandavison force-pushed the document-client-worker-interceptors branch from 64121d6 to 8232d0d Compare July 15, 2025 03:30
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.

Minor stuff. Would like to see this for every lang and make it into docs.temporal.io.

README.md Outdated
Comment on lines 1355 to 1356
that you wish to use to effect your modifications. Then, pass a list containing an instance of your `worker.Interceptor`
class as the `interceptors` argument of `Client.connect()`.
Copy link
Member

Choose a reason for hiding this comment

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

pass a list containing an instance of your worker.Interceptor class as the interceptors argument of Client.connect().

Only client interceptors should be passed to client connect. If it also implements worker interceptor, then that ends up applying to worker too, but you can't just pass worker interceptors to client connect.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah thanks, yes that section wasn't quite there. I've pushed an update.

README.md Outdated
that you wish to use to effect your modifications. Then, pass a list containing an instance of your `worker.Interceptor`
class as the `interceptors` argument of `Client.connect()`.

You can also pass worker interceptors as the `interceptor` argument to the
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
You can also pass worker interceptors as the `interceptor` argument to the
You can also pass worker interceptors as the `interceptors` argument to the


1. Outbound client calls, such as `start_workflow()`, `signal_workflow()`, `list_workflows()`, `update_schedule()`, etc.

2. Inbound workflow calls: `execute_workflow()`, `handle_signal()`, `handle_update_handler()`, etc
Copy link
Member

Choose a reason for hiding this comment

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

Ug, I just realized we called it handle_update_handler instead of handle_update here, oh well too late to change now

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That was a deliberate design decision -- to match handle_update_validator(). Pretty sure you were involved in that decision.

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 would personally have preferred handle_update()

Copy link
Member

@cretz cretz Jul 15, 2025

Choose a reason for hiding this comment

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

K, yeah I guess I regret my choice here if that is what happened. Luckily I didn't replicate it in .NET or Ruby.

README.md Outdated

4. Inbound call to execute an activity: `execute_activity()`

5. Outbound activity calls: `info()` and `hearbeat()`
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
5. Outbound activity calls: `info()` and `hearbeat()`
5. Outbound activity calls: `info()` and `heartbeat()`

@dandavison dandavison merged commit 1fec723 into main Jul 15, 2025
16 checks passed
@dandavison dandavison deleted the document-client-worker-interceptors branch July 15, 2025 15:23
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.

2 participants