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

Feature: Moved all the connector config values into a seperate file #4395

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

subhajit20
Copy link
Contributor

Type of Change

  • Bugfix
  • New feature
  • Enhancement
  • Refactoring
  • Dependency updates
  • Documentation
  • CI/CD

Description

Additional Changes

  • This PR modifies the API contract
  • This PR modifies the database schema
  • This PR modifies application configuration/environment variables

Motivation and Context

How did you test it?

Checklist

  • I formatted the code cargo +nightly fmt --all
  • I addressed lints thrown by cargo clippy
  • I reviewed the submitted code
  • I added unit tests for my changes where possible
  • I added a CHANGELOG entry if applicable

@subhajit20 subhajit20 requested a review from a team as a code owner April 18, 2024 14:18
@subhajit20 subhajit20 changed the title Moved all the connector config values into a seperate file Feature: Moved all the connector config values into a seperate file Apr 18, 2024
@subhajit20 subhajit20 force-pushed the FEATURE/move_connector_config_separate_file branch from 75905f0 to ca3b3ca Compare April 18, 2024 14:22
@subhajit20 subhajit20 force-pushed the FEATURE/move_connector_config_separate_file branch from 7bccba5 to 7011357 Compare April 18, 2024 14:32
@Narayanbhat166
Copy link
Member

Hi @subhajit20, There are few more changes required to complete this.

  • Since the connector configs are moved to a separate file now, changes need to be made to read these configs from the separate file. You can have a look at how the application config is read by having a look at this function

    pub fn with_config_path(config_path: Option<PathBuf>) -> ApplicationResult<Self> {
    // Configuration values are picked up in the following priority order (1 being least

  • You have made the changes only to loadtest/development.toml
    This file is read when we are running the application in loadtest mode .There is another file which is read by the application during startup ( while running the application locally ), this file is development.toml

  • Once you have made the changes, you can check whether the configs are read by checking the startup config log line. This will be visible once you have run the cargo r command.
    Screenshot 2024-04-22 at 12 09 11 PM

@subhajit20 subhajit20 force-pushed the FEATURE/move_connector_config_separate_file branch from 5d1a0c9 to 902b787 Compare April 25, 2024 05:02
@subhajit20
Copy link
Contributor Author

Hi @subhajit20, There are few more changes required to complete this.

  • Since the connector configs are moved to a separate file now, changes need to be made to read these configs from the separate file. You can have a look at how the application config is read by having a look at this function
    pub fn with_config_path(config_path: Option<PathBuf>) -> ApplicationResult<Self> {
    // Configuration values are picked up in the following priority order (1 being least
  • You have made the changes only to loadtest/development.toml
    This file is read when we are running the application in loadtest mode .There is another file which is read by the application during startup ( while running the application locally ), this file is development.toml
  • Once you have made the changes, you can check whether the configs are read by checking the startup config log line. This will be visible once you have run the cargo r command.
    Screenshot 2024-04-22 at 12 09 11 PM

I have created new config file for the develop startup as well, can you guide me what should I do right now? I am little confused

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.

None yet

2 participants