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

Support .env config from other folders #201

Open
orweis opened this issue Jan 11, 2022 · 9 comments · May be fixed by #715
Open

Support .env config from other folders #201

orweis opened this issue Jan 11, 2022 · 9 comments · May be fixed by #715
Labels
bug Something isn't working good first issue Good for newcomers help wanted Extra attention is needed

Comments

@orweis
Copy link
Contributor

orweis commented Jan 11, 2022

Describe the bug
No easy way of setting .env
OPAL uses decouple to parse configuration from .env and .ini files (As part of Confi)
BY default it looks for them in the folder of the importing module - we should add support for a configurable folder or at least the current-directory

@orweis orweis added bug Something isn't working good first issue Good for newcomers help wanted Extra attention is needed labels Jan 11, 2022
@money8203
Copy link
Contributor

Hey @orweis can you explain me the issue and the changes needed, I don't know the tech stack but definitely want to give it a try if possible

@orweis
Copy link
Contributor Author

orweis commented Jan 16, 2023

Hi @money8203 ,
Sure thing! We'd love some help with these : )

Confi is OPAL's configuration framework (homebrewed) - https://github.com/permitio/opal/blob/master/packages/opal-common/opal_common/confi/confi.py

It loads config values for OPAL from 3 potential sources - ENV-var, cmd line parameters, and config files (.env and .ini).
The decouple is the one reading the files and it's AutoConfig class has the setting of where to look for the files.

We'd like to have the default location it searches for config files in as the current working dir, and maybe also an ENV_VAR that controls that path

@money8203
Copy link
Contributor

money8203 commented Jan 17, 2023

Some suggestions I found on how we can fix the bug:

  • Add support for a configurable folder: We can add an option for the user to specify the location of the config files. We can do this by adding a command line argument or an environment variable that lets the user specify the path to the config files.

  • Add support for the current-directory: We can make the current-directory as the default location that Confi looks for the config files. This way, if the user does not specify a different location, Confi will look for the files in the current-directory by default.

  • Add an ENV_VAR that controls the path of the config files: We can add an environment variable that lets the user specify the path to the config files. This way, the user can set the path to the config files using an environment variable instead of specifying it on the command line.

Here's a sample of updated code for the above feedback:

import os

class Confi:
    def __init__(self, prefix=None, is_model=True) -> None:
        self._is_model = is_model
        self._prefix = prefix
        self._counter = 0
        self._entries: Dict[str, ConfiEntry] = OrderedDict()
        self._delayed_entries: Dict[str, ConfiDelay] = OrderedDict()
        self._delayed_defaults: Dict[str, ConfiEntry] = OrderedDict()

    def _load_config_file(self, file_path):
        """
        Loads the config file located at file_path
        """
        # add code to read and parse config file

    def load_config(self):
        """
        Loads the config files from the specified location
        """
        # check for location specified by user
        config_path = os.environ.get("CONFI_PATH")
        if config_path:
            self._load_config_file(config_path)
        else:
            # use current directory as default location
            current_dir = os.getcwd()
            self._load_config_file(current_dir)

  1. The updated code creates a new class called "Confi" that has a constructor that sets some initial values and creates some instance variables. The class also contains a new method called "_load_config_file" which accepts a file path as its parameter, this method is responsible for reading and parsing the config files.

  2. The class also contains a new method called "load_config" which is responsible for loading the config files from the specified location. This method first checks for the location specified by the user through the environment variable "CONFI_PATH", if it exists, it will use that location to load the config files. If the environment variable is not set, it will use the current directory as the default location to load the config files.

  3. This allows the users to have more control over their config files, they can specify the location of their config files using the environment variable "CONFI_PATH" or if not set, the default location will be the current directory.

@orweis
Copy link
Contributor Author

orweis commented Jan 17, 2023

Hi I think you got the right direction here, but note the trick is with "_load_config_file" as this bit of the code is within decouple, and not part of OPAL's direct code

@Rajat0002
Copy link

Hi @orweis
One approach is to modify the decouple library to accept an additional parameter that specifies the location of the configuration files. This could be done by adding a new argument to the config function that takes the path to the config files as input.

@orweis
Copy link
Contributor Author

orweis commented Jan 22, 2023

Hi @Rajat0002 :)
As we don't want to fork decouple - I think we can work with inheritance -
In confi we import and use config from decouple, which is an instance of AutoConfig
Instead we can inherit from AutoConfig and create another class - which overrides the internal SUPPORTED static member or the method _find_file

https://github.com/HBNetwork/python-decouple/blob/9cc2f7e3b2f82d306b981975da2925f05a816f51/decouple.py#L188

@LucasOcampos
Copy link

Hey @orweis, is this issue still happening? If so, could I give it a try? Also, I've read the thread here but I might need more help on the matter.

@orweis
Copy link
Contributor Author

orweis commented Dec 2, 2024

Hey @orweis, is this issue still happening? If so, could I give it a try? Also, I've read the thread here but I might need more help on the matter.

Hi sorry for the late reply. while not an urgent issue this is still open.

@LucasOcampos
Copy link

LucasOcampos commented Dec 2, 2024

@orweis no worries, I'll see what I can do here and might ask for help if needed! It's my first contribution to the project so I'd love to get some grasp of the structure through this task!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
4 participants