issue #80: remove louis reference from pyproject.toml
#89
Conversation
k-allagbe
changed the title
pyproject.toml
This reverts commit 4f7cfc1.
|
Reported the test failure issue here #90 |
…ub.com/ai-cfia/ailab-db into 80-fix-package-setup-referencing-louis
k-allagbe
force-pushed
the
80-fix-package-setup-referencing-louis
branch
from
d8a9362 to
1d19d59
Compare
k-allagbe
force-pushed
the
80-fix-package-setup-referencing-louis
branch
from
1d19d59 to
cf88406
Compare
| api_key=os.environ["OPENAI_API_KEY"], | ||
| azure_endpoint=f"https://{os.environ['AZURE_OPENAI_SERVICE']}.openai.azure.com", |
There was a problem hiding this comment.
This is a personal preference, but I find defining environment variables has a constant is very helpful for change management. Here's an example:
OPENAI_API_KEY = 'OPENAI_API_KEY'
AZURE_OPENAI_SERVICE = 'AZURE_OPENAI_SERVICE'There was a problem hiding this comment.
I agree. I would suggest a good round of refactoring on this codebase, especially since the delay issues have been fixed to a tolerable amount.
|
|
||
| # https://learn.microsoft.com/en-us/azure/cognitive-services/openai/how-to/embeddings?tabs=python | ||
|
|
||
|
|
||
| def safe_get(key): | ||
| value = os.environ.get(key) | ||
| if not value: | ||
| raise Exception(f"Environment variable {key} not defined") |
There was a problem hiding this comment.
I suggest the use of KeyError or a custom exception instead of Exception to be more precise on the nature of the exception.
There was a problem hiding this comment.
Agreed. I would suggest a round of refactor. #92
| @@ -33,7 +29,6 @@ def raise_error(message): | |||
|
|
|||
| def connect_db(): | |||
| """Connect to the postgresql database and return the connection.""" | |||
| logger.info(f"Connecting to {LOUIS_DSN}") | |||
| connection = psycopg.connect( | |||
| conninfo=LOUIS_DSN, | |||
There was a problem hiding this comment.
Should we change the secrets from [LOUIS_...] to [AI_LAB_...] if we want to remove all references?
There was a problem hiding this comment.
Good point. I would suggest to rewrite packages in a way that they don't force the env var naming on their users because it could lead to conflicts. For instance, if I already had POSTGRES_URI in my repo, ailab-db would force me to rename it to LOUIS_DSN or have a duplicate of the same value. What I suggest is to pass the value to the function that needs it.
Fixing this would even greatly improve testability. I suggest this be part of #92.
Closes #80:
pyproject.tomlreferencing louis