Skip to content

Conversation

Spikhalskiy
Copy link
Contributor

@Spikhalskiy Spikhalskiy commented Jan 26, 2023

What?

Added a concept of Worker Name. Worker Name defaults to Worker Task Queue, but can be explicitly set. Worker Name can be used by auto-discovery annotations instead of Task Queues.

Why?

Application that incorporates Task Queue based Versioning strategy may choose to use explicit Worker names to reference because it adds a level of indirection.
This way Task Queue name is specified only once in the config and can be easily changed when needed, while all the auto-discovery annotations reference the Worker by its static name.

Closes #1550

@Spikhalskiy Spikhalskiy force-pushed the sb-worker-name branch 5 times, most recently from 25b79f0 to 6347608 Compare January 26, 2023 18:24
@Spikhalskiy Spikhalskiy marked this pull request as ready for review January 26, 2023 18:24
@Spikhalskiy Spikhalskiy force-pushed the sb-worker-name branch 2 times, most recently from 3650b2d to b3c7cc2 Compare January 26, 2023 20:16
Copy link
Member

@cretz cretz left a comment

Choose a reason for hiding this comment

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

This change makes sense.

When worker versioning comes around, they may want to run the same worker for different versions. I think users then will definitely want to put their workflows/activities on differently-named workers with the same task queue but maybe different versions.

for (String workerName : annotation.workers()) {
Worker worker = workers.getByName(workerName);
if (worker == null) {
throw new BeanDefinitionValidationException(
Copy link
Member

Choose a reason for hiding this comment

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

I almost think we should encourage this indirection just because I find the "worker with name and task queue required" exception to be clearer than creating a worker with default settings when you don't find one for a task queue (you won't know a mistyped task queue name is in your annotation until too late).

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 think that default workers are mostly "quick project to try things" type of deal. Production users will have a config section. But I think it's fine to make workers auto-discoverable for people who want things up and running as easy as possible.

Worker newWorker = workerDescriptor.getWorker();
Worker existingWorker = workersByName.get(workerDescriptor.getName());
if (existingWorker != null
&& !existingWorker.getTaskQueue().equals(newWorker.getTaskQueue())) {
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't name be globally unique regardless of task queue?

Copy link
Contributor Author

@Spikhalskiy Spikhalskiy Jan 27, 2023

Choose a reason for hiding this comment

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

Yeah, they are.
If the task queue is the same - it means there is no NAME conflict. I made this check to prevent situations when users give workers conflicting names. Worker with the same name has the same task queue.
The following check address a situation if we try to add another worker for the same task queue. (And it's a code bug, callers of this method should prevent it).

Let me rearrange code to make my intention here more clear.

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.

Dynamic & interpolated taskQueues

2 participants