Skip to content

Remove redundant logging of found config file#1669

Merged
Aniruddh25 merged 8 commits intomainfrom
fixLogging
Aug 30, 2023
Merged

Remove redundant logging of found config file#1669
Aniruddh25 merged 8 commits intomainfrom
fixLogging

Conversation

@Aniruddh25
Copy link
Copy Markdown
Collaborator

@Aniruddh25 Aniruddh25 commented Aug 29, 2023

Why make this change?

What is this change?

  • Store the environment based config file name found at construction time in the property ConfigFileName.
  • Remove Console writeline statements while checking for existence of file since this leads to multiple logs - one from CLI before starting the engine and second from within the engine itself when calling TryPargeConfig
  • Add a single log of the loaded file in CLI

How was this tested?

  • Manually, using the start command of CLI as well as triggering the engine directly with argument: --ConfigFileName
  1. Starting with default config:
    Before:
    image

After:
image

  1. Starting with user provided config:
    Before:
    image

After:
image

  1. Trying to start with missing file name - same behvior as before
    image

@Aniruddh25 Aniruddh25 enabled auto-merge (squash) August 30, 2023 04:28
@abhishekkumams
Copy link
Copy Markdown
Contributor

does this PR also address the issue here ?

@Aniruddh25
Copy link
Copy Markdown
Collaborator Author

does this PR also address the issue here ?

yes it does

@Aniruddh25 Aniruddh25 linked an issue Aug 30, 2023 that may be closed by this pull request
1 task
@Aniruddh25 Aniruddh25 merged commit e3b65c9 into main Aug 30, 2023
@Aniruddh25 Aniruddh25 deleted the fixLogging branch August 30, 2023 15:05
Aniruddh25 added a commit that referenced this pull request Aug 30, 2023
- With the overhaul of config system PR #1402 , we started seeing
redundant logs about which config file is being used.

- Store the environment based config file name found at construction
time in the property `ConfigFileName`.
- Remove Console writeline statements while checking for existence of
file since this leads to multiple logs - one from CLI before starting
the engine and second from within the engine itself when calling
TryPargeConfig
- Add a single log of the loaded file in CLI
- Also catch exception for incorrect connection string parsing

- Manually, using the `start` command of CLI as well as triggering the
engine directly with argument: `--ConfigFileName`

1. Starting with default config:
**Before**:

![image](https://github.com/Azure/data-api-builder/assets/3513779/a29d4aff-19db-49c7-a745-b6bbdb5c62cc)

**After**:

![image](https://github.com/Azure/data-api-builder/assets/3513779/62f65222-de59-4525-b0b1-ead407d43b4e)

2. Starting with user provided config:
**Before**:

![image](https://github.com/Azure/data-api-builder/assets/3513779/b762c873-ade6-47bb-b094-44960176d90f)

**After**:

![image](https://github.com/Azure/data-api-builder/assets/3513779/2f977a72-bfdf-43fb-9cc9-806ea717b546)

3. Trying to start with missing file name - same behvior as before

![image](https://github.com/Azure/data-api-builder/assets/3513779/5a80f62e-8a06-4e7c-aa22-c120dade6801)
Aniruddh25 added a commit that referenced this pull request Aug 30, 2023
Cherry picking #1669 

Original description:

## Why make this change?
- With the overhaul of config system PR #1402 , we started seeing
redundant logs about which config file is being used.

## What is this change?

- Store the environment based config file name found at construction
time in the property `ConfigFileName`.
- Remove Console writeline statements while checking for existence of
file since this leads to multiple logs - one from CLI before starting
the engine and second from within the engine itself when calling
TryPargeConfig
- Add a single log of the loaded file in CLI

## How was this tested?
- Manually, using the `start` command of CLI as well as triggering the
engine directly with argument: `--ConfigFileName`

1. Starting with default config:
**Before**:

![image](https://github.com/Azure/data-api-builder/assets/3513779/a29d4aff-19db-49c7-a745-b6bbdb5c62cc)

**After**:

![image](https://github.com/Azure/data-api-builder/assets/3513779/62f65222-de59-4525-b0b1-ead407d43b4e)

2. Starting with user provided config:
**Before**:

![image](https://github.com/Azure/data-api-builder/assets/3513779/b762c873-ade6-47bb-b094-44960176d90f)

**After**:

![image](https://github.com/Azure/data-api-builder/assets/3513779/2f977a72-bfdf-43fb-9cc9-806ea717b546)

3. Trying to start with missing file name - same behvior as before

![image](https://github.com/Azure/data-api-builder/assets/3513779/5a80f62e-8a06-4e7c-aa22-c120dade6801)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: "--config" option is not honoring the user provided config file

3 participants