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

fix init command with environment variable #1541

Merged
merged 12 commits into from
Jul 10, 2023

Conversation

abhishekkumams
Copy link
Contributor

Why make this change?

  • Closes [Bug]: DAB init doesn't honor DAB_ENVIRONMENT variable #1527
    • dab init doesn't create file with the environment Value, and always create default config if not specified explicitly.
    • For ex: if DAB_ENVIRONMENT is set to Development, it should create dab-config.Development.json, which is not the case.

What is this change?

  • Updating the logic to create config file giving precedence to environment value, also making sure error is thrown if the file is not present already.

How was this tested?

  • Unit Tests

Sample Request(s)

dab init --database-type mssql
above command will create:

  1. DAB_ENVIRONMENT = null
    dab-config.json
  2. DAB_ENVIRONMENT = Development
    dab-config.Development.json

@abhishekkumams abhishekkumams marked this pull request as ready for review June 22, 2023 04:42
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 and suggestions for simplification

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.

Thanks for fixing! also for addressing comment, very clean. Appreciate the ability to see full file path in the logs, will help us/PMs/customers troubleshoot if necessary!

Copy link
Contributor

@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.

LGTM, after fixing some corner cases.

@abhishekkumams abhishekkumams merged commit 6aad7db into main Jul 10, 2023
@abhishekkumams abhishekkumams deleted the dev/abhishekkuma/fix_dab_init branch July 10, 2023 09:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working cli
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: DAB init doesn't honor DAB_ENVIRONMENT variable
4 participants