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

Add Operation InputType and OutputType helper methods #21

Merged
merged 2 commits into from
Oct 4, 2024

Conversation

bergundy
Copy link
Contributor

@bergundy bergundy commented Oct 4, 2024

Useful helper methods, we need this in the Temporal Go SDK to validate inputs when starting an operation from a workflow.

@@ -36,6 +38,13 @@ func (r operationReference[I, O]) Name() string {
return string(r)
}

func (r operationReference[I, O]) IsInputAssignable(i any) bool {
Copy link

@cretz cretz Oct 4, 2024

Choose a reason for hiding this comment

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

Not sure this needs to be on every operation reference (nor does IsOutputAssignable). If you had a InputType() reflect.Type that could maybe make a bit more sense, but anyone can obtain the generic of the operation reference via reflection. IMO having a helper for Go reflection is unnecessary for this public API.

Can just update the Temporal side to use reflection if it needs to check whether something is assignable, same as it would for any generic API it would need to do a similar check for. I wouldn't expect types with generics to provide this helper.

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 don't think it's possible to get the generic input type in Go. I tried this before and ended up blocked on golang/go#54393.

Maybe there's another way you're aware of that I'm not.

Copy link

@cretz cretz Oct 4, 2024

Choose a reason for hiding this comment

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

I see, my mistake. I think InputType() reflect.Type and OutputType() reflect.Type would be better general-purpose methods to add (or Types() (input reflect.Type, output reflect.Type)).

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 works for me.

@bergundy bergundy changed the title Add Operation.IsInputAssignable helper method Add Operation InputType and OutputType helper methods Oct 4, 2024
@@ -36,6 +40,16 @@ func (r operationReference[I, O]) Name() string {
return string(r)
}

func (operationReference[I, O]) InputType() reflect.Type {
var zero [0]I
return reflect.TypeOf(zero).Elem()
Copy link

Choose a reason for hiding this comment

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

Suggested change
return reflect.TypeOf(zero).Elem()
return reflect.TypeOf((*I)(nil)).Elem()

This may also work (untested), but what you have is fine/cheap too, no need to change.

@bergundy bergundy merged commit 29a4cb7 into nexus-rpc:main Oct 4, 2024
3 checks passed
@bergundy bergundy deleted the is-input-assignable branch October 4, 2024 23:43
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