Skip to content

AER-4020 Added feature to override queue configuration settings with environment variables#119

Open
Hilbrand wants to merge 2 commits intoaerius:mainfrom
Hilbrand:override-config
Open

AER-4020 Added feature to override queue configuration settings with environment variables#119
Hilbrand wants to merge 2 commits intoaerius:mainfrom
Hilbrand:override-config

Conversation

@Hilbrand
Copy link
Copy Markdown
Member

This change will make it possible to override any value in the queue configuration settings.

There are some additional breaking changes:

  • In the queue configuration files the client queue configuration has been changed from an list of client queues to a map of client queues with the client queue name as key. This makes it easier to override values for specific client queues as the name of the queue can be used. With the list the index would have to be used, but that is somewhat arbitrary and difficult to validate. For now the old way is still supported.
  • Overriding complete queue file configuration for a single worker queue with an environment variable has been removed in favor of overriding single values.
  • Also removed some of the test configuration files as those are not actually used, but where template files for actually configuration used as such in the past.

…nt variables

This change will make it possible to override any value in the queue configuration settings.

There are some additional breaking changes:
- In the queue configuration files the client queue configuration has been changed from an list of client queues to a map of client queues with the client queue name as key.
  This makes it easier to override values for specific client queues as the name of the queue can be used. With the list the index would have to be used, but that is somewhat arbitrary and difficult to validate.
  For now the old way is still supported.
- Overriding complete queue file configuration for a single worker queue with an environment variable has been removed in favor of overriding single values.
- Also removed some of the test configuration files as those are not actually used, but where template files for actually configuration used as such in the past.
Comment on lines -40 to +34
private static final String ENV_PREFIX = "AERIUS_PRIORITY_TASK_SCHEDULER_";
private static final String PREFIX = "AERIUS_PTS_";
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Assume we haven't been using the old prefix in our deployements so far?
(using different prefix makes sense I think, as the way it should be filled in is a bit different as well)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I made the prefix shorter, because the whole name can become rather large already when overriding nested variables. This keeps it a bit more readable.

/**
* @deprecated Replaced by clientQueues for easier overriding with environment variables. Because list requires index, while map can use key name.
*/
@Deprecated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It's not exactly deprecated, as the clientQueues also end up in the queues list? Might make more sense to fill clientQueues based on queues and work with that map from now on if you want to get rid of this list in the future?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It's not that I would prefer to get rid of the list. This is is preferred because otherwise the name of the queue is both key and as field in the object. The object is passed so it makes sense to keep the name on the object. Also the name of the queue is rewritten to be a full name (in the configuration on the prefix is actually configured).
Therefore I refactored it based on your comment to add an extra subclass specific for reading the configuration. The subclass contains the map, and the original class with queues is kept as it was original (So no deprecation). After reading the config the map is put in the list. Than the list becomes the data being used in the rest of the application.

Comment on lines +60 to +61
case "AERIUS_PTS_WORKERQUEUENAME" -> "ops";
case "AERIUS_PTS_MAXWORKERSAVAILABLE" -> "100";
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If you're using this, and taskmanager is configured to have multiple schedulers, doesn't it override the value for all schedulers?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This is not the same as used in task manager itself. Because the prefix is passed to the code tested here. In the task manager itself it is AERIUS_PTS_<queue name>. So this code in the unit test is a bit deceptive.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ah right, because it's using PREFIX + fileSchedule.getWorkerQueueName() in PriorityTaskSchedulerFileHandler. Does make WORKERQUEUENAME a weird one to override, as then the order matters?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yes overriding WORKERQUEUENAME would not go as expected as it uses the original name for the environment variables. In the unit test it's more of a case that it's possible, but in practice I don't think it should every be overridden.

…g client queue configuration

That way the map variant is not part of the actual configuration file used in the Task Manager because the list variant is preferred to be used throughout the code, but the map variant is preferred for configuration as it allows easier overriding values with environment variables.
@Hilbrand Hilbrand requested a review from BertScholten April 17, 2026 11:06
Copy link
Copy Markdown
Member

@BertScholten BertScholten left a comment

Choose a reason for hiding this comment

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

LGTM

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