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

Don't proceed further if domain does not exist #29

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

adilhafeez
Copy link

@adilhafeez adilhafeez commented Oct 17, 2019

This code change allows program to terminate as soon as it sees that domain does not exists. With this change error message is more clear. For example see this error message with this code change,

➜  cadence-samples git:(fail_fast) ✗ ./bin/helloworld -m worker
2019-10-16T23:15:01.957-0700	INFO	common/sample_helper.go:65	Logger created.
2019-10-16T23:15:01.958-0700	DEBUG	common/factory.go:131	Creating RPC dispatcher outbound	{"ServiceName": "cadence-frontend", "HostPort": "127.0.0.1:7933"}
2019-10-16T23:15:01.970-0700	FATAL	common/sample_helper.go:83	Domain doesn't exist	{"Domain": "samples-domain1", "error": "EntityNotExistsError{Message: Domain samples-domain1 does not exist.}"}
github.com/uber-common/cadence-samples/cmd/samples/common.(*SampleHelper).SetupServiceConfig
	/Users/ahafeez/src/go/src/github.com/uber-common/cadence-samples/cmd/samples/common/sample_helper.go:83
main.main
	/Users/ahafeez/src/go/src/github.com/uber-common/cadence-samples/cmd/samples/recipes/helloworld/main.go:40
runtime.main
	/usr/local/Cellar/go/1.12.9/libexec/src/runtime/proc.go:200
➜  cadence-samples git:(fail_fast) ✗

And without this code change here is the same error,

➜  cadence-samples git:(master) ✗ ./bin/helloworld -m worker
2019-10-16T23:14:33.330-0700	INFO	common/sample_helper.go:65	Logger created.
2019-10-16T23:14:33.330-0700	DEBUG	common/factory.go:131	Creating RPC dispatcher outbound	{"ServiceName": "cadence-frontend", "HostPort": "127.0.0.1:7933"}
2019-10-16T23:14:33.339-0700	INFO	common/sample_helper.go:83	Domain doesn't exist	{"Domain": "samples-domain1", "error": "EntityNotExistsError{Message: Domain samples-domain1 does not exist.}"}
2019-10-16T23:14:33.380-0700	ERROR	internal/internal_worker.go:244	domain does not exist	{"Domain": "samples-domain1", "TaskList": "helloWorldGroup", "WorkerID": "9488@ahafeez-mbp143@helloWorldGroup", "WorkerType": "DecisionWorker", "domain": "samples-domain1", "error": "EntityNotExistsError{Message: Domain samples-domain1 does not exist.}"}
go.uber.org/cadence/internal.verifyDomainExist.func1
	/Users/ahafeez/src/go/pkg/mod/go.uber.org/[email protected]/internal/internal_worker.go:244
go.uber.org/cadence/internal/common/backoff.Retry
	/Users/ahafeez/src/go/pkg/mod/go.uber.org/[email protected]/internal/common/backoff/retry.go:98
go.uber.org/cadence/internal.verifyDomainExist
	/Users/ahafeez/src/go/pkg/mod/go.uber.org/[email protected]/internal/internal_worker.go:262
go.uber.org/cadence/internal.(*workflowWorker).Start
	/Users/ahafeez/src/go/pkg/mod/go.uber.org/[email protected]/internal/internal_worker.go:357
go.uber.org/cadence/internal.(*aggregatedWorker).Start
	/Users/ahafeez/src/go/pkg/mod/go.uber.org/[email protected]/internal/internal_worker.go:1004
github.com/uber-common/cadence-samples/cmd/samples/common.(*SampleHelper).StartWorkers
	/Users/ahafeez/src/go/src/github.com/uber-common/cadence-samples/cmd/samples/common/sample_helper.go:135
main.startWorkers
	/Users/ahafeez/src/go/src/github.com/uber-common/cadence-samples/cmd/samples/recipes/helloworld/main.go:21
main.main
	/Users/ahafeez/src/go/src/github.com/uber-common/cadence-samples/cmd/samples/recipes/helloworld/main.go:44
runtime.main
	/usr/local/Cellar/go/1.12.9/libexec/src/runtime/proc.go:200
2019-10-16T23:14:33.380-0700	ERROR	common/sample_helper.go:137	Failed to start workers.	{"error": "EntityNotExistsError{Message: Domain samples-domain1 does not exist.}"}
github.com/uber-common/cadence-samples/cmd/samples/common.(*SampleHelper).StartWorkers
	/Users/ahafeez/src/go/src/github.com/uber-common/cadence-samples/cmd/samples/common/sample_helper.go:137
main.startWorkers
	/Users/ahafeez/src/go/src/github.com/uber-common/cadence-samples/cmd/samples/recipes/helloworld/main.go:21
main.main
	/Users/ahafeez/src/go/src/github.com/uber-common/cadence-samples/cmd/samples/recipes/helloworld/main.go:44
runtime.main
	/usr/local/Cellar/go/1.12.9/libexec/src/runtime/proc.go:200
panic: Failed to start workers

goroutine 1 [running]:
github.com/uber-common/cadence-samples/cmd/samples/common.(*SampleHelper).StartWorkers(0xc00030e480, 0xc00002a100, 0xf, 0x182fdf6, 0xf, 0x0, 0x0, 0x0, 0x0, 0x0, ...)
	/Users/ahafeez/src/go/src/github.com/uber-common/cadence-samples/cmd/samples/common/sample_helper.go:138 +0x2fd
main.startWorkers(...)
	/Users/ahafeez/src/go/src/github.com/uber-common/cadence-samples/cmd/samples/recipes/helloworld/main.go:21
main.main()
	/Users/ahafeez/src/go/src/github.com/uber-common/cadence-samples/cmd/samples/recipes/helloworld/main.go:44 +0x24b

@CLAassistant
Copy link

CLAassistant commented Oct 17, 2019

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ adilhafeez
❌ Groxx
You have signed the CLA already but the status is still pending? Let us recheck it.

Copy link
Contributor

@Groxx Groxx left a comment

Choose a reason for hiding this comment

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

We should probably consider doing a specific check for this case... but yeah, this is an improvement either way. Describing the domain is in fact blocking for the helper.

@Groxx
Copy link
Contributor

Groxx commented Nov 1, 2023

aah, sorry - I didn't notice the CLA was needed because you're a new contributor.

@adilhafeez: if you click the details on that license/cla check, we just need that filled in and then it should run tests / pass / allow merging.

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.

None yet

4 participants