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: Drilled Config #107

Merged
merged 22 commits into from
Sep 11, 2024
Merged

Feature: Drilled Config #107

merged 22 commits into from
Sep 11, 2024

Conversation

heemankv
Copy link
Contributor

@heemankv heemankv commented Sep 4, 2024

This PR resolves #104 and #110.

Introduces :

  • Config drilled from parent to child functions, making it more pure and removing dirty access of global config.
  • New TestConfigBuilder with sync config method and only build async method.
  • Creates new for alert_client and generalises Region to AWS_REGION removing usage of AWS_SNS_REGION.
  • Ensures no excess access to AWS_ENV_VARS other than init_config and testconfigbuilder.
  • Adoption of ProviderConfig - removing get_settings and adding get_settings_or_panic.

@heemankv heemankv added the enhancement New feature or request label Sep 4, 2024
@heemankv heemankv self-assigned this Sep 4, 2024
@heemankv heemankv changed the title Feature/drilled config Feature: Drilled Config Sep 4, 2024
@coveralls
Copy link

coveralls commented Sep 4, 2024

Coverage Status

coverage: 64.87% (-0.7%) from 65.576%
when pulling 2b0fb3c on feature/drilled-config
into 369ccbe on main.

crates/orchestrator/src/config.rs Outdated Show resolved Hide resolved
crates/orchestrator/src/config.rs Outdated Show resolved Hide resolved
crates/orchestrator/src/config.rs Outdated Show resolved Hide resolved
crates/orchestrator/src/tests/config.rs Outdated Show resolved Hide resolved
crates/orchestrator/src/tests/config.rs Outdated Show resolved Hide resolved
crates/orchestrator/src/tests/config.rs Outdated Show resolved Hide resolved
crates/orchestrator/src/tests/config.rs Outdated Show resolved Hide resolved
crates/orchestrator/src/tests/workers/proving/mod.rs Outdated Show resolved Hide resolved
crates/orchestrator/src/tests/workers/proving/mod.rs Outdated Show resolved Hide resolved
crates/orchestrator/src/tests/config.rs Outdated Show resolved Hide resolved
crates/orchestrator/src/tests/config.rs Outdated Show resolved Hide resolved
* update: better alert impl

* ignore: empty comment to trigger CI

* update: TestConfigBuilder object name changes

* update: removed new_from_env() from AWSSNS

* ignore: empty comment to trigger CI
@heemankv heemankv linked an issue Sep 6, 2024 that may be closed by this pull request
Copy link
Contributor

@EvolveArt EvolveArt left a comment

Choose a reason for hiding this comment

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

just a few comments

crates/orchestrator/src/tests/config.rs Show resolved Hide resolved
crates/orchestrator/src/tests/config.rs Outdated Show resolved Hide resolved
crates/orchestrator/src/tests/config.rs Show resolved Hide resolved
crates/orchestrator/src/tests/config.rs Show resolved Hide resolved
Copy link
Contributor

@EvolveArt EvolveArt left a comment

Choose a reason for hiding this comment

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

LGTM

crates/orchestrator/src/config.rs Outdated Show resolved Hide resolved
crates/orchestrator/src/jobs/da_job/mod.rs Outdated Show resolved Hide resolved
crates/utils/src/settings/env.rs Outdated Show resolved Hide resolved
crates/orchestrator/src/tests/config.rs Show resolved Hide resolved
crates/orchestrator/src/config.rs Show resolved Hide resolved
@heemankv heemankv merged commit bbcf7b0 into main Sep 11, 2024
9 checks passed
@heemankv heemankv deleted the feature/drilled-config branch September 11, 2024 14:23
k4kratik pushed a commit that referenced this pull request Sep 18, 2024
* update: drilled config

* update: TestConfigBuilder with configurations

* chore: lint fixes

* chore: lint fixes #2

* update: Non-Arc Impl for TestConfigBuilder

* update: New TestConfigBuilder accomodating changed on TestCases

* update: uncomment fft tests

* update: PR review changes #1

* update: optimised init_<service> functions

* Improvement/better alert (#114)

* update: better alert impl

* ignore: empty comment to trigger CI

* update: TestConfigBuilder object name changes

* update: removed new_from_env() from AWSSNS

* ignore: empty comment to trigger CI

* update: starknet dummy provider optimisiation

* update: starknet dummy provider + Conversion optimisiation

* update: added default for ConfigType

* update: mod implement_client added

* update: adaptation to provider-config

* fix : e2e test

* update: fix integration tests

* fix: PR reviews fixed

* update: reworking Provider config to be an ARC

---------

Co-authored-by: Arun Jangra <arunjangra1001@gmail.com>
ocdbytes added a commit that referenced this pull request Oct 16, 2024
* update: drilled config

* update: TestConfigBuilder with configurations

* chore: lint fixes

* chore: lint fixes #2

* update: Non-Arc Impl for TestConfigBuilder

* update: New TestConfigBuilder accomodating changed on TestCases

* update: uncomment fft tests

* update: PR review changes #1

* update: optimised init_<service> functions

* Improvement/better alert (#114)

* update: better alert impl

* ignore: empty comment to trigger CI

* update: TestConfigBuilder object name changes

* update: removed new_from_env() from AWSSNS

* ignore: empty comment to trigger CI

* update: starknet dummy provider optimisiation

* update: starknet dummy provider + Conversion optimisiation

* update: added default for ConfigType

* update: mod implement_client added

* update: adaptation to provider-config

* fix : e2e test

* update: fix integration tests

* fix: PR reviews fixed

* update: reworking Provider config to be an ARC

---------

Co-authored-by: Arun Jangra <arunjangra1001@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Better Alerts Add code from parallel test cases branch to Main
5 participants