fix: always disable CDN testing #264
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
CDN testing should always be on — dealbot tests SPs directly, not services that wrap SPs. This removes the env var, config plumbing, and documentation for ENABLE_CDN_TESTING, and hardcodes enableCDN to true. Closes #169
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request removes the ENABLE_CDN_TESTING environment variable configuration and hardcodes CDN testing to always be enabled during deal creation. The changes remove the configuration toggle from validation schemas, config interfaces, environment files, and documentation, while setting enableCDN = true in the deal service. However, the CDN addon strategy and related infrastructure remain in place.
Changes:
- Removed
ENABLE_CDN_TESTINGfrom Joi validation schema,IBlockchainConfiginterface, and config loader - Hardcoded
enableCDN = trueinDealService.getTestingDealOptions() - Removed environment variable from
.env.example, kustomize configmap, and all documentation - Updated test mocks to remove
enableCDNTestingproperty and changed test assertions to expectenableCDN: true
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| apps/backend/src/config/app.config.ts | Removed ENABLE_CDN_TESTING from Joi schema, IBlockchainConfig interface, and loadConfig() function |
| apps/backend/src/deal/deal.service.ts | Hardcoded enableCDN = true in getTestingDealOptions() method |
| apps/backend/src/deal/deal.service.spec.ts | Removed enableCDNTesting from mock configs and changed assertion from expect.any(Boolean) to true |
| apps/backend/src/wallet-sdk/wallet-sdk.service.spec.ts | Removed enableCDNTesting: false from base test configuration |
| apps/backend/.env.example | Removed ENABLE_CDN_TESTING=true environment variable |
| apps/backend/README.md | Removed ENABLE_CDN_TESTING row from blockchain configuration table |
| docs/environment-variables.md | Removed entire ENABLE_CDN_TESTING documentation section |
| kustomize/overlays/local/backend-configmap-local.yaml | Removed ENABLE_CDN_TESTING: "false" and associated comment |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
SgtPooki
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Remove the
ENABLE_CDN_TESTINGenvironment variable and always disable CDN testingin deal creation.
Problem
The
ENABLE_CDN_TESTINGflag controlled whether CDN was randomly enabled (50%chance) during deal creation. However, dealbot should be testing storage providers
directly — not services that wrap SPs. Having a toggle for CDN testing adds
unnecessary configuration complexity and inconsistent test behavior.
Solution
ENABLE_CDN_TESTINGfrom the Joi validation schema, config interface(
IBlockchainConfig), and config loaderenableCDN = falseinDealService.getTestingDealOptions().env.example, kustomize configmap, and alldocumentation
Notes
enableCDNparameter still flows throughDealPreprocessingConfigand theCDN addon strategy — only the config toggle was removed
creation behavior
Fixes #169