Adding in additional header to determine a more stable isolation-group #1252
Conversation
| @@ -26,6 +26,8 @@ import ( | |||
| "fmt" | |||
| "time" | |||
|
|
|||
| "go.uber.org/cadence/internal/common/isolationgroup" | |||
There was a problem hiding this comment.
did you run go imports or go fmt?
There was a problem hiding this comment.
I tried to, not entirely sure if/if there's still a format issue
There was a problem hiding this comment.
Unfortunately, neither go imports nor go fmt will not join import groups, so if you have them separated by a new line, they will still be in separate groups. Also, Goland's default behavior is to add imports in a new group separating with a new line, which is unfortunate. I think monorepo folks have recently introduced a change that should enforce imports to follow Go Uber style guide. We could adjust later on.
There was a problem hiding this comment.
goimports might have a way to force that 🤔. if it does, I'd totally be in favor of it - special-case imports can still be commented, and if we have order-dependent imports then I'm happy to say "absolutely not, remove those init funcs and replace them with something sane"
| } | ||
|
|
||
| func (w *workflowServiceIsolationGroupWrapper) ListDomains(ctx context.Context, request *shared.ListDomainsRequest, opts ...yarpc.CallOption) (*shared.ListDomainsResponse, error) { | ||
| opts = append(opts, w.getIsolationGroupIdentifier()) |
There was a problem hiding this comment.
given how regular this pattern is, might be worth adopting gowrap to reduce minor mistakes now and in maintenance.
we already have it in the server, it'd be fairly easy to add here: https://github.com/uber/cadence/blob/e2a2a940030ec62a0749a4f4d8c86100305220c2/client/frontend/interface.go#L32
I'm not going to investigate codegen in this PR, though it certainly feel's like it should have it at this point. Cleaned up the test though |
What changed?
Why?
How did you test it?
Potential risks