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

feat(infra): select provider from environment variables #4916

Conversation

p-fernandez
Copy link
Contributor

What change does this PR introduce?

Allows to select from environment variables the provider for the Workflow Engine and the Cache Service. In the case of not being set up by the new method it will fall back to the retro-compatible current way.

Why was this change needed?

To simplify the configuration of the providers for the Workflow Engine and the Cache service and allow the users to choose their own providers.

Other information (Screenshots)

New environment variables to set up the providers:

process.env.CACHE_PROVIDER_ID;
process.env.CACHE_HOST;
process.env.CACHE_PORTS;
process.env.CACHE_USERNAME;
process.env.CACHE_PASSWORD;
process.env.WORKFLOW_PROVIDER_ID;
process.env.WORKFLOW_HOST;
process.env.WORKFLOW_PORTS;
process.env.WORKFLOW_USERNAME;
process.env.WORKFLOW_PASSWORD;

Potentially we can add the rest of the configuration. That could be done further down the line.

@p-fernandez p-fernandez self-assigned this Nov 29, 2023
Copy link

linear bot commented Nov 29, 2023

NV-3132 The system should allow an admin to pass in different env for each cluster

What?

Currently, if the is cluster env ariable is turned on then both the cache and the queue cluster are turned on. We want to be able to set up a configuration that allows us to configure the environment variables for each separately and are specific to the function instead of the cluster.

cache_provider=MemoryDB
cache_host=""

cache_port=""

queue_provider=AzureRedis

queue_host=""

queue_port=""

Why? (Context)

This will allow use to be more transparent and clean with our implementation of our redis instances to clearly communicate what environment variables set up what.

Definition of Done

@p-fernandez p-fernandez force-pushed the nv-3132-the-system-should-allow-an-admin-to-pass-in-different-env branch from dd8eeb8 to 2eebe12 Compare November 29, 2023 17:19
@@ -52,6 +58,54 @@ export class CacheInMemoryProviderService {
};
}

private getCacheProvider(): IEnvironmentConfigOptions {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
private getCacheProvider(): IEnvironmentConfigOptions {
private getCacheConfiguration(): IEnvironmentConfigOptions {


if (envOptions) {
redisConfig = {
host: convertStringValues(envOptions.host),
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to allow override partially? for example providing only envOptions.port and all the other params will be used from the env variables.

Base automatically changed from nv-3197-move-provider-selection-to-workflow-and-cache-in-memory to refactor-memory-provider-init December 28, 2023 13:14
@djabarovgeorge djabarovgeorge marked this pull request as ready for review December 28, 2023 13:14
@djabarovgeorge djabarovgeorge merged commit f1b6614 into refactor-memory-provider-init Dec 28, 2023
21 of 23 checks passed
@djabarovgeorge djabarovgeorge deleted the nv-3132-the-system-should-allow-an-admin-to-pass-in-different-env branch December 28, 2023 13: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