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 add support for env variables #286

Merged
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -14,3 +14,4 @@ dist/

# python venv
venv
robyn.code-workspace
7 changes: 7 additions & 0 deletions definitions.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import os

# Path to the root of the project
ROOT_DIR = os.path.dirname(os.path.abspath(__file__))

Copy link
Member

Choose a reason for hiding this comment

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

We can either move the contents of the file to import_vars only or if you like to keep the constants in a separate file, we can create a submodule.

Let me know if you need any help with that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the idea was that this file defines where the root of the project is, because if i tried to define the root in a nested file it would be dependent on the path of the file I guess

Copy link
Member

Choose a reason for hiding this comment

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

We can use Pathlib(https://docs.python.org/3/library/pathlib.html) to resolve the path. And I feel that we can move these constants and env_populator to a module of their own as the constants are only being used in the env_populator.py.

# Path to the environment variables
CONFIG_PATH = os.path.join(ROOT_DIR, 'configuration.conf')
Copy link
Member

Choose a reason for hiding this comment

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

I convention for the configuration file that I had in my mind was something like robyn.env. What do you think about it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i think robyn.env is great i've changed it to that

33 changes: 33 additions & 0 deletions robyn/env_populator.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
import os
from definitions import CONFIG_PATH

# parse the configuration file returning a list of tuples (key, value) containing the environment variables
def parser(config_path=CONFIG_PATH):
"""Parse the configuration file"""
with open(config_path, 'r') as f:
for line in f:
if line.startswith('#'):
continue
yield line.strip().split('=')


# check for the environment variables set in cli and if not set them
def load_vars(variables = None):
"""Main function"""

variables = parser()

if variables is None:
return

for var in variables:
if var[0] in os.environ:
print("Variable {} already set".format(var[0]))
Copy link
Member

Choose a reason for hiding this comment

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

We can use logger module here. Similar to (https://github.com/sansyrox/robyn/blob/main/robyn/__init__.py#L21) .

We will be able to control the logs using the log-level.

Second, instead of format, we can use f-strings as we support Python >=3.7. https://docs.python.org/3/tutorial/inputoutput.html#tut-f-strings

Copy link
Contributor Author

Choose a reason for hiding this comment

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

should logs be in debug level or info level? and I changed the syntax such that f-strings are used instead

Copy link
Member

Choose a reason for hiding this comment

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

@Shending-Help , these logs should be debug level in my opinion.

Copy link
Member

Choose a reason for hiding this comment

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

Or info works too. There is a very thin line and doesn't really matter that much.

continue
else:
os.environ[var[0]]=var[1]
print(var[0], var[1])