-
Notifications
You must be signed in to change notification settings - Fork 4
Updated codebase to use QuestDB instead of streaming files from S3 #2
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
Updated codebase to use QuestDB instead of streaming files from S3 #2
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This pull request migrates the backtesting codebase from streaming files from S3 to using QuestDB as a data source. The changes are extensive and involve significant architectural modifications.
Key Changes
- Upgraded from .NET 8.0 to .NET 9.0 across all projects
- Replaced S3-based file streaming with QuestDB direct database queries
- Refactored environment variable management from mock-based to injection-based pattern
- Removed web server and UI components
- Updated test infrastructure to use JSON configuration files instead of mocks
Reviewed Changes
Copilot reviewed 150 out of 155 changed files in this pull request and generated 30 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test.csproj | Upgraded target framework to net9.0 |
| tests/resources/environmentVariables.json | Added JSON configuration file for test environment variables |
| tests/backtesting/TestEnvironmentSetup.cs | Refactored to load environment from JSON and use injection pattern |
| tests/backtesting/UtilitesTests/ExtensionTests.cs | Updated tests to use new environment setup |
| tests/backtesting/StrategyTests/RandomStrategyTests.cs | Modernized test implementation with proper AAA pattern |
| tests/backtesting/Shared.cs | Added EnvironmentJson class for JSON deserialization |
| tests/backtesting/ReportingTests/ReportingTests.cs | Updated tests to use environment injection |
| src/utilities/utilities.csproj | Upgraded to net9.0, added MathNet.Numerics package |
| src/utilities/StandardDeviation.cs | Added new statistical calculation utilities |
| src/utilities/ServiceExtension.cs | Removed environment variable dependency, added PropertyCopier utility |
| src/utilities/ReportEnvironmentVariables.cs | Added new utility for reporting environment variables |
| src/utilities/EnvironmentVariables_v2.cs | Implemented new enum-based environment variable system |
| src/utilities/ConsoleLogger.cs | Updated to use new environment variable system |
| src/utilities/MedianExtensions.cs | Added median calculation extensions |
| src/utilities/LinearRegression.cs | Added linear regression utilities |
| src/strategies/strategies.csproj | Upgraded to net9.0 and Microsoft.Extensions.Hosting 8.0.1 |
| src/strategies/Random.cs | Updated to use new strategy definition pattern |
| src/strategies/BaseStrategy.cs | Removed environment variables and web notification dependencies |
| src/backtesting/backtesting-engine.csproj | Upgraded to net9.0, added Npgsql for QuestDB connectivity |
| src/backtesting/Program.cs | Major refactoring to use QuestDB instead of S3 streaming |
| src/backtesting/Ingest/IngestFromQuestDB.cs | New ingestion layer for QuestDB |
| src/backtesting/Operations/Positions.cs | Updated to use new environment variable system |
| src/backtesting/TradeManagement/CloseOrder.cs | Simplified trade closing logic |
| src/backtesting/Analysis/Reporting.cs | Enhanced reporting with statistical metrics |
| src/Models/models.csproj | Upgraded to net9.0, added MemoryPack serialization |
| src/Models/StrategyDefinition.cs | Added new strategy configuration model |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "LAMBDA_LOG": "FALSE", | ||
| "SYSTEM_LOG": "TRUE", | ||
| "CONSOLE_LOG": "TRUE", | ||
| "SCALING_FACRTOR": "TestEnvironmentSetup,1000;", |
Copilot
AI
Nov 14, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Spelling error: "SCALING_FACRTOR" should be "SCALING_FACTOR"
tests/backtesting/Shared.cs
Outdated
|
|
||
| // TradingVariables | ||
| public required string STRATEGY { get; set; } | ||
| public required string SCALING_FACRTOR { get; set; } |
Copilot
AI
Nov 14, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Spelling error: "SCALING_FACRTOR" should be "SCALING_FACTOR". This typo appears consistently throughout the codebase.
| public enum TradingVariables | ||
| { | ||
| STRATEGY, | ||
| SCALING_FACRTOR, |
Copilot
AI
Nov 14, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Spelling error: "SCALING_FACRTOR" should be "SCALING_FACTOR"
| /// <param name="exclusiveEnd">The exclusive exclusiveEnd index.</param> | ||
| /// <param name="rsquared">The r^2 value of the line.</param> | ||
| /// <param name="yintercept">The y-intercept value of the line (i.e. y = ax + b, yintercept is b).</param> | ||
| /// <param name="slope">The slop of the line (i.e. y = ax + b, slope is a).</param> |
Copilot
AI
Nov 14, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Spelling error in comment: "slop" should be "slope"
src/backtesting/Program.cs
Outdated
| EnvironmentVariables.Inject(LoggingVariables.LAMBDA_LOG, BACKTESTING_VARIABLES.LAMBDA_LOG.ToString()); | ||
| EnvironmentVariables.Inject(LoggingVariables.SYSTEM_LOG, BACKTESTING_VARIABLES.SYSTEM_LOG.ToString()); | ||
|
|
||
| EnvironmentVariables.Inject(TradingVariables.SCALING_FACRTOR, "AUDUSD,10000;EURUSD,10000;GBRIDXGBP,1;GBPUSD,10000;NZDUSD,10000;USDJPY,100;GBPJPY,100;EURJPY,100;USDCAD,10000;FRAIDXEUR,1;EURGBP,10000;USA500IDXUSD,1;AUSIDXAUD,1;USDCHF,10000;XAUUSD,1;XAGUSD,1;USATECHIDXUSD,1;EURCHF,10000;DEUIDXEUR,1;USA30IDXUSD,1;LIGHTCMDUSD,1;JPNIDXJPY,1;BRENTCMDUSD,1;"); |
Copilot
AI
Nov 14, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Spelling error: "SCALING_FACRTOR" should be "SCALING_FACTOR"
| } | ||
|
|
||
| private static IEnumerable<DictionaryEntry>? entries = Environment.GetEnvironmentVariables().Cast<DictionaryEntry>(); | ||
| private static Dictionary<String, String> envCache = []; |
Copilot
AI
Nov 14, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Field 'envCache' can be 'readonly'.
| catch (Exception ex) | ||
| { | ||
| result = ex.Message; | ||
| Console.WriteLine(ex); | ||
| ConsoleLogger.Log($"Unexpected error: {result}"); | ||
| await SendStackException(ex); | ||
|
|
||
| } |
Copilot
AI
Nov 14, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generic catch clause.
| catch (Exception ex) | ||
| { | ||
| ConsoleLogger.Log($"Error during cleanup: {ex.Message}"); | ||
| } |
Copilot
AI
Nov 14, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generic catch clause.
| catch (Exception ex) | ||
| { | ||
| throw new TradingException($"Error retrieving OHLC data: {ex.Message}", ex); | ||
| } |
Copilot
AI
Nov 14, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generic catch clause.
| if (tradingObjects.openTrades.Count > 0) | ||
| { | ||
| return tradingObjects.openTrades.Select(x => x.Value); | ||
| } | ||
| else | ||
| { | ||
| return []; | ||
| } |
Copilot
AI
Nov 14, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Both branches of this 'if' statement return - consider using '?' to express intent better.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 150 out of 155 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| Host=localhost; | ||
| Port=8812; | ||
| Username=admin; | ||
| Password=quest; |
Copilot
AI
Nov 15, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hardcoded database credentials are a security risk. The password 'quest' should be loaded from environment variables or a secure configuration source instead of being hardcoded in the connection string.
| count++; // to track results | ||
| if (count % 5 != 0) | ||
| { | ||
| continue; // skip every 10th tick |
Copilot
AI
Nov 15, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment says "skip every 10th tick" but the code actually skips 4 out of every 5 ticks (only processes when count % 5 == 0). This comment is misleading and should be corrected to accurately reflect the filtering logic.
|




No description provided.