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

Start worker even when no workflow/activity registered #539

Merged
merged 3 commits into from
Sep 17, 2021

Conversation

yiminc
Copy link
Member

@yiminc yiminc commented Sep 16, 2021

What was changed

Do not skip worker start when registry is empty.

Why?

There are uses cases where registry happen after worker is started, for example in integration tests that you want to start worker as part of test setup where you don't know what to register yet.
This check only make sense when the registry was done via golang's init() func. Now registry is per worker and can be done after worker is started. The current behavior of silently skip worker start is inconvenient.

@@ -159,7 +159,7 @@ func (s *CacheEvictionSuite) TestResetStickyOnEviction() {
s.service.EXPECT().ResetStickyTaskQueue(gomock.Any(), gomock.Any()).DoAndReturn(mockResetStickyTaskQueue).Times(1)

client := internal.NewServiceClient(s.service, nil, internal.ClientOptions{})
workflowWorker := internal.NewAggregatedWorker(client, "taskqueue", worker.Options{})
workflowWorker := internal.NewAggregatedWorker(client, "taskqueue", worker.Options{LocalActivityWorkerOnly: true})
Copy link
Member Author

Choose a reason for hiding this comment

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

This test requires workflow worker only and the mock service is setup accordingly. If we start activity worker, the mock would complain. The test works because this test does not register any activity and as side effect, activity worker does not start. With the change in this PR, activity worker would start and this test failed. Fix is to explicitly disable activity worker.

@yiminc yiminc enabled auto-merge (squash) September 17, 2021 03:26
@yiminc yiminc disabled auto-merge September 17, 2021 03:41
@yiminc yiminc merged commit 16bb227 into temporalio:master Sep 17, 2021
@yiminc yiminc deleted the start_worker branch September 19, 2021 18:15
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