ai-cfia / fertiscan-backend

Fertiscan backend
MIT License
1 stars 0 forks source link

As a developer, I want our unit tests to correctly retrieve environment variables from both the local .env file and OS environment variables. #64

Closed Endlessflow closed 2 weeks ago

Endlessflow commented 2 weeks ago

Context

Currently, our testing workflows consistently fail when attempting to run unit tests. The primary hypothesis is that our tests are retrieving environment variables from a local .env file instead of using the OS environment variables. This discrepancy leads to failures in our GitHub Actions because the required keys are missing from the .env file but are present as GitHub secrets.

This issue impacts the reliability of our continuous integration pipeline, causing delays and confusion among developers when tests pass locally but fail in CI/CD environments.

Acceptance Criteria

snakedye commented 2 weeks ago

We already use the OS environment variables. What dotenv does is load the variables in .env as OS environment variables.

The testing workflow also doesn't use Github Secrets AFAIK. It's still a bit foggy but what I recall from what @SonOfLope told me is that what is currently used to handle secrets isn't really fit with the way workflows are run so they're migrating to something Azure related so everything can be in the same place.

SonOfLope commented 2 weeks ago

We already use the OS environment variables. What dotenv does is load the variables in .env as OS environment variables.

The testing workflow also doesn't use Github Secrets AFAIK. It's still a bit foggy but what I recall from what @SonOfLope told me is that what is currently used to handle secrets isn't really fit with the way workflows are run so they're migrating to something Azure related so everything can be in the same place.

It is using github secrets as of yesterday.

SonOfLope commented 2 weeks ago

We already use the OS environment variables. What dotenv does is load the variables in .env as OS environment variables.

The testing workflow also doesn't use Github Secrets AFAIK. It's still a bit foggy but what I recall from what @SonOfLope told me is that what is currently used to handle secrets isn't really fit with the way workflows are run so they're migrating to something Azure related so everything can be in the same place.

The problem is you are 'only' loading from .env. In production, these variables will be available as environment variables and not in a .env file. As such, you will need to fetch them directly from the OS environment variables.

snakedye commented 2 weeks ago

It is using github secrets as of yesterday.

Thanks, so I suppose all that's left is to put the variables there?

SonOfLope commented 2 weeks ago

It is using github secrets as of yesterday.

Thanks, so I suppose all that's left is to put the variables there?

You also need to fetch the variables in the code. Example: Nachet doing both loading from .env and directly fetching the variables : https://github.com/ai-cfia/nachet-backend/blob/main/app.py

snakedye commented 2 weeks ago

The problem is you are 'only' loading from .env. In production, these variables will be available as environment variables and not in a .env file. As such, you will need to fetch them directly from the OS environment variables.

load_env doesn't clear all the other environment variables and then load the ones in .env. It only loads the variables in .env so if the file is empty or a variable isn't present there, it will no effect on the other variables. I'm also not loading from .env, I'm loading from the OS.

I just made a test locally and it can pick up on env vars passed through the OS.

SonOfLope commented 2 weeks ago

it seems AZURE_OPENAI_KEY is missing.... @Endlessflow

@snakedye can you add the variable to the repository secrets with right value. It should then fix the pipeline. Also looking at the code, you guys are already doing both loading from dotenv and from environment.

snakedye commented 2 weeks ago

@SonOfLope Done image

SonOfLope commented 2 weeks ago

@Endlessflow seems like theres more env variables than what you stated : image

@snakedye could you please add secrets for all existing env variables haha

snakedye commented 2 weeks ago

I added all the others. image

SonOfLope commented 2 weeks ago

image Congrats everyone. This issue can be closed.

snakedye commented 2 weeks ago

GG.