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

[Feature] Enable Settings even for Core plugin component #946

Open
scicco opened this issue Oct 17, 2024 · 2 comments
Open

[Feature] Enable Settings even for Core plugin component #946

scicco opened this issue Oct 17, 2024 · 2 comments
Labels
enhancement New feature or request

Comments

@scicco
Copy link

scicco commented Oct 17, 2024

Is your feature request related to a problem? Please describe.

It should be nice to have a settings section for the core plugin where we can specify the custom variables used for the cat. When opening the settings, the default values will be set as the corresponding ENV variable.

Example:

variable name: log_level
default value: get_env('LOG_LEVEL') or 'INFO'

get_env

def get_env(name):
)

default_value of log_level:

"CCAT_LOG_LEVEL": "INFO",

With this change, we can add additional settings without the need to introduce additional ENV variables.

This can avoid any breaking changes for all deployments already in production but let us define additional settings even without introducing other ENVs

some use cases:

Describe the solution you'd like

A user can override those settings runtime. When the user saves the settings, those values are written inside the cat data folder, maybe in a new settings.json file.

On startup, the code checks for the existence of settings.json. If no file is present it will be filled with the default values and then saved.

For newer values without an ENV, we can modify the settings.json default template to include that default values

We can then use the watch library to listen for changes to that file, and then restart the server if needed

Describe alternatives you've considered

Change settings without the watch library involved. In this case, the server must be restarted manually to apply the new values

@scicco scicco added the enhancement New feature or request label Oct 17, 2024
@matteocacciola
Copy link

matteocacciola commented Oct 19, 2024

I see the advantages of managing the current env variables in a centralized data storage without the necessity to redeploy.

On the other hand, you need to be sure to have the latest updated value, when you read an env variable. For instance, triggering such a flow to save the updated values in memory every time an env variable is changed could be inefficient, when the app runs in a cluster. Then, every time you want to use an env variable you are forced to grab it from the centralized data storage. Do you agree with me? This could affect the performances.

Then, in order to appreciate the advantages of such a solution, I would ask few questions:

  1. How frequent this kind of changes could be?
  2. What kind of scenario you have in mind when the introduction of a new env variable does not require changes in the codebase (e.g. implementation of new features)?

@scicco
Copy link
Author

scicco commented Oct 21, 2024

  1. It depends on the project and the variable goal. For example, regarding lowering recall threshold #912 maybe you need to fine-tune the values based on your needs, and you have to try different values till you are satisfied
  2. The project has already many ENV variables, maybe in the future it would be nice to be able to provide a way to specify a new setting with a default value without adding a new ENV variable to cover that. This means that you will have to change the default settings.json template instead of hardcoding values in the codebase.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants