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

Update upstream protos #655

Merged

Conversation

antlai-temporal
Copy link
Contributor

What was changed

Update api protos to the latest.

Why?

Need SDK workflow metadata message to implement __temporal_getWorkflowMetadata query

@antlai-temporal antlai-temporal requested a review from a team as a code owner December 11, 2023 05:33
option ruby_package = "Temporalio::Api::Sdk::V1";
option csharp_namespace = "Temporalio.Api.Sdk.V1";

// The name of the query to retrieve this information is `__temporal_getWorkflowMetadata`.
Copy link
Member

Choose a reason for hiding this comment

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

I kinda wish I paid more attention at temporalio/api#336, I would have suggested the get be removed as it's implied with queries and to use underscores to separate words like __stack_trace did.

Oh well, it'd be nice if you fixed but I suppose not required since I technically marked approved.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

At this point is just a comment in the repo, so I could use __temporal_workflow_metadata in the implementation, and the next time we touch the protos we fix the comment.

Copy link
Member

Choose a reason for hiding this comment

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

@antlai-temporal - I would love this! I think it'd be much better.

@antlai-temporal antlai-temporal merged commit d3d3ad8 into temporalio:master Dec 11, 2023
7 checks passed
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.

3 participants