AER-4020 Added feature to override queue configuration settings with environment variables#119
AER-4020 Added feature to override queue configuration settings with environment variables#119Hilbrand wants to merge 2 commits intoaerius:mainfrom
Conversation
…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.
| private static final String ENV_PREFIX = "AERIUS_PRIORITY_TASK_SCHEDULER_"; | ||
| private static final String PREFIX = "AERIUS_PTS_"; |
There was a problem hiding this comment.
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)
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
| case "AERIUS_PTS_WORKERQUEUENAME" -> "ops"; | ||
| case "AERIUS_PTS_MAXWORKERSAVAILABLE" -> "100"; |
There was a problem hiding this comment.
If you're using this, and taskmanager is configured to have multiple schedulers, doesn't it override the value for all schedulers?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Ah right, because it's using PREFIX + fileSchedule.getWorkerQueueName() in PriorityTaskSchedulerFileHandler. Does make WORKERQUEUENAME a weird one to override, as then the order matters?
There was a problem hiding this comment.
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.
This change will make it possible to override any value in the queue configuration settings.
There are some additional breaking changes: