Skip to content

Provide client in activity context #740

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 4 commits into from
Jul 16, 2025
Merged

Conversation

dandavison
Copy link
Contributor

@dandavison dandavison commented Jan 18, 2025

Fixes #721

@dandavison dandavison requested a review from a team as a code owner January 18, 2025 04:30
@dandavison dandavison marked this pull request as draft January 18, 2025 04:42
@dandavison dandavison force-pushed the 203-activity-client-instance branch 3 times, most recently from 962135c to 3164f69 Compare January 23, 2025 22:48
@dandavison dandavison marked this pull request as ready for review January 23, 2025 22:48
@dandavison
Copy link
Contributor Author

This needs a bit more work -- as things stand it's attempting to pickle the client in the context in tests of process-based executor pools (which fails when it tries to pickle the RuntimeRef), but I wonder if that means it is attempting to share the client across threads in thread-based executor pools.

@cretz
Copy link
Member

cretz commented Jun 27, 2025

I'd be ok for now raising an exception for if the activity.client() is invoked in def activites (so async def activities only for now)

@dandavison dandavison force-pushed the 203-activity-client-instance branch 3 times, most recently from bd3b25e to 2f04c55 Compare July 16, 2025 01:44
@dandavison dandavison force-pushed the 203-activity-client-instance branch from 2f04c55 to 3c2c1bc Compare July 16, 2025 01:50
@dandavison
Copy link
Contributor Author

Rebased and ready for review again.

Makes the worker's connected client.Client available in activities as activity.client().

As discussed above, the client is only available in async def activities.

@dandavison dandavison force-pushed the 203-activity-client-instance branch from 3c2c1bc to 08e0223 Compare July 16, 2025 02:02
@dandavison dandavison force-pushed the 203-activity-client-instance branch from fc616a6 to 7ef37f6 Compare July 16, 2025 12:13
@dandavison dandavison merged commit e6f6f91 into main Jul 16, 2025
16 checks passed
@dandavison dandavison deleted the 203-activity-client-instance branch July 16, 2025 13: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.

[Feature Request] Provide client from activity context
2 participants