Skip to content

Conversation

@abhishekkumams
Copy link
Contributor

@abhishekkumams abhishekkumams commented May 16, 2023

Why make this change?

What is this change?

  • Using the package DotNetEnv
  • Now, users can maintain a local file .env to manage environment variables instead of setting environment variables in the system.
  • If .env file is used, it will take precedence over system environment variables.

How was this tested?

  • Unit Tests

NOTE:

  • This feature is to enhance local development experience, and it works when used via DAB CLI.

QnA

  1. Where should the .env file be created?
    -> For .env file to be considered, it should be present in the same directory where the dab commands are executed.

  2. Does it overwrite the existing environment variables?
    -> No, it doesn't update any existing variables. It just gives precedence to the variable defined in the file over the system defined variable in case it is present in both places.

@abhishekkumams abhishekkumams self-assigned this May 16, 2023
@abhishekkumams abhishekkumams added enhancement New feature or request cli labels May 16, 2023
@seantleonard
Copy link
Contributor

PR description:

If .env file is not used, it will take precedence over system environment variables.

should the line read: "If .env file is used" ?

Copy link
Contributor

@seantleonard seantleonard left a comment

Choose a reason for hiding this comment

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

Some questions

@abhishekkumams abhishekkumams marked this pull request as ready for review June 1, 2023 09:25
@abhishekkumams abhishekkumams dismissed stale reviews from Aniruddh25, seantleonard, and aaronpowell June 5, 2023 12:49

updated

Copy link
Contributor

@seantleonard seantleonard left a comment

Choose a reason for hiding this comment

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

some additional suggestions.

Copy link
Contributor

@ayush3797 ayush3797 left a comment

Choose a reason for hiding this comment

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

Some nits. LGTM otherwise!

Copy link
Collaborator

@Aniruddh25 Aniruddh25 left a comment

Choose a reason for hiding this comment

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

Not sure why we are testing DotNetEnv.Env.Load externally.

@abhishekkumams
Copy link
Contributor Author

Not sure why we are testing DotNetEnv.Env.Load externally.

We have tests that verifies DotNetEnv.Env.Load internally. Line:125(Environment)

we have some external tests as well to just verify the method ParseConfigJsonAndReplaceEnvVariables as calling this directly won't load the variables from .env file.

Loading from .env file happens at the initialization step of DAB CLI, so that every cli command is synced with the environment variables.

Hope this addresses your concerns?

@Aniruddh25
Copy link
Collaborator

Not sure why we are testing DotNetEnv.Env.Load externally.

We have tests that verifies DotNetEnv.Env.Load internally. Line:125(Environment)

we have some external tests as well to just verify the method ParseConfigJsonAndReplaceEnvVariables as calling this directly won't load the variables from .env file.

Loading from .env file happens at the initialization step of DAB CLI, so that every cli command is synced with the environment variables.

Hope this addresses your concerns?

Thanks for the clarifications, could you comment the additional scenarios which this test is handling.

@abhishekkumams
Copy link
Contributor Author

Not sure why we are testing DotNetEnv.Env.Load externally.

We have tests that verifies DotNetEnv.Env.Load internally. Line:125(Environment)
we have some external tests as well to just verify the method ParseConfigJsonAndReplaceEnvVariables as calling this directly won't load the variables from .env file.
Loading from .env file happens at the initialization step of DAB CLI, so that every cli command is synced with the environment variables.
Hope this addresses your concerns?

Thanks for the clarifications, could you comment the additional scenarios which this test is handling.

Sure, For Example we are:

  • testing that there are no breaks using the original method for using environment variable.
  • testing scenarios where we have variables defined in both system and .env file

@abhishekkumams abhishekkumams merged commit 0b3e3ab into main Jun 9, 2023
@abhishekkumams abhishekkumams deleted the dev/abhishekkuma/add_support_for_env_file branch June 9, 2023 09:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cli enhancement New feature or request

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Add support for providing environment var/values in a .env file read by dotenv.net

7 participants