Add sample for stand alone nexus operations#456
Conversation
| require.Eventually(t, func() bool { | ||
| var execErr error | ||
| handle, execErr = nexusClient.ExecuteOperation(ctx, opName, input, options) | ||
| return execErr == nil |
There was a problem hiding this comment.
I think we should fix the need to retry for tests before we launch SANO. It's going to be annoying to use.
You should also only filter out not found errors here.
Evanthx
left a comment
There was a problem hiding this comment.
Asking some questions, not sure how many are just me being new still!
|
|
||
| replace github.com/cactus/go-statsd-client => github.com/cactus/go-statsd-client/v5 v5.0.0 | ||
|
|
||
| replace go.temporal.io/sdk => /Users/quinnklassen/Documents/Code/sdk-go |
There was a problem hiding this comment.
I know you said this wasn't ready for checkin, but figured it was worth pointing out these two replace commands!
|
|
||
| func main() { | ||
| // The client is a heavyweight object that should be created once per process. | ||
| c, err := client.Dial(envconfig.MustLoadDefaultClientOptions()) |
There was a problem hiding this comment.
The other Nexus samples are using "options.ParseClientOptionFlags(os.Args[1:])" before client.Dial, should this as well for consistent behavior?
There was a problem hiding this comment.
Yeah bit on the fence about this since the ParseClientOptionFlags(os.Args[1:]) was only added because we didn't have envconfig now that we do we should probably just lean on the since very other sample uses it. For consistency we should probably remove ParseClientOptionFlags from other samples and just lean on envconfig
|
|
||
| // Create a NexusClient bound to the endpoint and service. | ||
| // The endpoint must be pre-created on the server (see README). | ||
| nexusClient, err := c.NewNexusClient(client.NexusClientOptions{ |
There was a problem hiding this comment.
This is an odd one when looking at other Nexus sample code in here. I went to the equivalent spot and there is no NewNexusClient, so I thought maybe that's new? But then I found it in workflows.go at the top level, which isn't in this sample.
Short version - I am reading the code to see what is different, and so get thrown off - and mostly I was reading this to learn how to use Nexus for standalone operations, and I can't tell what makes this standalone as opposed to self hosted!
There was a problem hiding this comment.
Other Nexus samples starter code is starting a workflow not a Nexus operation, the comparison isn't between standalone and self hosted, the comparison is between inside a workflow and outside a workflow
| } | ||
| log.Println("Hello result:", helloResult.Message) | ||
|
|
||
| // List Nexus operations using the base client (not NexusClient). |
There was a problem hiding this comment.
I am pretty sure this is just me being new to Nexus still but -- does the reader know what you are talking about here? Why are there two clients? Why would I use the base client? Base implies that the NexusClient has it as a base class, but clearly that's not the case!
There was a problem hiding this comment.
Base implies that the NexusClient has it as a base class
Go has no concept of classes or base class so I don't think that is a conclusion a Go developer would reach, that being said I welcome any suggestions to make the wording hear clearer.
There was a problem hiding this comment.
I decided to use actual types instead of just "base"
|
I'll take care of rebasing and cleaning this up while Quinn is OOO. |
4c3b013 to
e73488f
Compare
e73488f to
05d4736
Compare
| // executeWithRetry retries ExecuteOperation until the endpoint has propagated. | ||
| // The endpoint registry is eventually consistent. |
There was a problem hiding this comment.
Would be good to have a link to an issue or some other way to track when we can remove this once the server side changes are in.
Sample for stand alone nexus operations