Improvements. #3

Joseph-Matteo-Scorsone · 2025-11-06T15:36:00Z

Changed to comments, code conventions, written out readme, and got rid of unnecessary file.

jaredbroad · 2025-11-06T15:49:09Z

QuantConnect.DataBento/DataBentoDataProvider.cs

                {
-                    Log.Trace($"DataBentoProvider.UnsubscribeImpl(): Processing symbol {symbol}");
-
                    if (_client?.IsConnected == true)


this would silently fail to subscribe if not connected; you should either throw or try to reconnect

Private comment was to invert the If to reduce nesting.

jaredbroad · 2025-11-06T15:50:17Z

QuantConnect.DataBento/DataBentoDataProvider.cs


            // Connect to live gateway
-            Log.Trace("DataBentoProvider.Initialize(): Attempting async connection to DataBento live gateway");
            Task.Run(async () =>


Why async? Wouldn't this continue without successful connection? Test running this without an internet connection

jaredbroad · 2025-11-06T15:52:23Z

QuantConnect.DataBento/DataBentoDataProvider.cs

            if (!_potentialUnsupportedResolutionMessageLogged)
            {
                _potentialUnsupportedResolutionMessageLogged = true;
-                Log.Trace("DataBentoDataProvider.IsSupported(): " +


You deleted a bunch of informative logging here, why?
Statements like this "_potentialUnsupportedResolutionMessageLogged = true;" are intended to show an informative error message once. Why have the boolean if deleting the log?

Private comment was to delete the debugging logging. If there is informative logging to inform user about failures in subscription/application those will be needed

Throwing an exception is better than silently continuing as it will look like a code bug if not.

README.md

…ing. Changed all async to sync, only using Task to connect and get messages sync.

Romazes

Hey, your PR needs additional improvements.

Please, fix the project with Tests.
Add more [TestCases]

Romazes · 2025-11-17T20:40:55Z

QuantConnect.DataBento.Tests/QuantConnect.DataSource.DataBento.Tests.csproj

@@ -16,4 +15,4 @@
    <PackageReference Include="Microsoft.NET.Test.Sdk" Version="16.9.4" />
    <PackageReference Include="Microsoft.TestPlatform.ObjectModel" Version="16.9.4" />
    <PackageReference Include="QuantConnect.Algorithm" Version="2.5.*" />


I can not run test project.
You need remove it because the package QuantConnect.Lean.Engine already has it.

Hi could you please clarify this more? The way I wrote it is inside a Lean directory I cloned. I made this directory and all the databento code inside it in Lean. In the Algorithm.CSharp directory I wrote a test algorithm and set it in the config. From that I can easily run it in VScode with F5. I run the tests by dotnet test "path to the .csproj in the QuantConnect.DataBento.Test directory" That works for me, I just had to change some paths slightly in the config for the MarketHoursDatabase.FromDataFolder(); to work.

I went through the rest of your comments, thank you for them, and implemented the changes. I believe we're all good, I just haven't had time to run everything with live data due to the early close today. If you'd like we can talk more before I do another PR. Happy holidays!

I have run GetsHistory() Tests and got exception:

System.MissingMethodException : Method not found: 'Void CsvHelper.CsvReader..ctor(System.IO.TextReader, System.Globalization.CultureInfo)'.

That's a mean version conflict between dependencies of the CsvHelper NuGet package.
On the screen above, I showed you that some dependencies (in the Test project) have the CsvHelper package.

Romazes · 2025-11-17T20:41:54Z

QuantConnect.DataBento/DataBentoDataDownloader.cs

 using QuantConnect.Util;
 using QuantConnect.Configuration;
 using QuantConnect.Interfaces;
 using QuantConnect.Securities;


Please remove unused or unnecessary usings. ⬆

Romazes · 2025-11-17T20:43:13Z

QuantConnect.DataBento/DataBentoDataDownloader.cs

-        /// <inheritdoc cref="HttpClient"/>
        private readonly HttpClient _httpClient;
-
        private readonly string _apiKey;


Please remove.
This field is used only in .ctor.

Romazes · 2025-11-17T20:45:59Z

QuantConnect.DataBento/DataBentoDataDownloader.cs

-
        private readonly string _apiKey;
-
        private const decimal PriceScaleFactor = 1e-9m;


Please add an xml description of the const, such as a link to the documentation.

Romazes · 2025-11-17T20:48:54Z

QuantConnect.DataBento/DataBentoDataDownloader.cs

@@ -57,4 +55,4 @@
            _httpClient.DefaultRequestHeaders.Authorization = new AuthenticationHeaderValue("Basic", credentials);
        }


It appears to be an extra constructor, which we can remove.

Romazes · 2025-11-17T21:33:06Z

QuantConnect.DataBento/DataBentoDataDownloader.cs

        /// <summary>
        /// Maps a LEAN symbol to DataBento symbol format
        /// </summary>
        private string MapSymbolToDataBento(Symbol symbol)


You need create new class SymbolMapper : ISymbolMapper with methods:

GetBrokerageSymbol()

GetLeanSymbol()

example - PolygonSymbolMapper

Romazes · 2025-11-17T21:34:01Z

QuantConnect.DataBento/DataBentoDataDownloader.cs


        /// Class for parsing trade data from Databento
        /// Really used as a map from the http request to then get it in QC data structures
        private class DatabentoBar


Please create a folder called Models and place all classes there to make the code clearer, more readable, and less noisy.

Romazes · 2025-11-17T21:34:47Z

QuantConnect.DataBento/DataBentoDataDownloader.cs

            [Name("ts_event")]
            public long TimestampNanos { get; set; }

            public DateTime Timestamp => DateTimeOffset.FromUnixTimeSeconds(TimestampNanos / 1_000_000_000)


The Lean has a lot of extension use this one:

Time.UnixNanosecondTimeStampToDateTime()

Romazes · 2025-11-17T21:41:11Z

QuantConnect.DataBento/DataBentoDataDownloader.cs

                    {
                        yield return new Tick
                        {
                            Time = record.Timestamp,


We should return parameter Time in the Symbol timeZone parameter instead of UTC.

example: polygon

Romazes · 2025-11-17T21:42:36Z

QuantConnect.DataBento/DataBentoDataDownloader.cs

                .AddTicks((TimestampNanos % 1_000_000_000) / 100).UtcDateTime;

            [Name("open")]
            public decimal Open { get; set; }


Also, please ensure that when converting raw parameters into a DTO, you use the correct precision internally in the DTO.

Romazes

Thanks, I’ve included several remarks.
Please add test cases:

Connect
Disconnect
Subscribe with different resolution, tick type, symbols.
Unsubscriberibe

Romazes · 2025-11-18T17:21:53Z

QuantConnect.DataBento/DataBentoDataProvider.cs

+using QuantConnect.Data.Market;
 using QuantConnect.Util;
 using QuantConnect.Interfaces;
 using System.Collections.Generic;


Please, remove not used using ...s.

Romazes · 2025-11-18T17:23:36Z

QuantConnect.DataBento/DataBentoDataProvider.cs

 namespace QuantConnect.Lean.DataSource.DataBento
 {
    /// <summary>
    /// Implementation of Custom Data Provider


Please, write a great and associated description. 😎

Romazes · 2025-11-18T17:26:37Z

QuantConnect.DataBento/DataBentoDataProvider.cs

-        /// <summary>
-        /// <inheritdoc cref="IDataAggregator"/>
-        /// </summary>
        private readonly IDataAggregator _dataAggregator = Composer.Instance.GetExportedValueByTypeName<IDataAggregator>(


We should init it in Initialize() by pattern:

_dataAggregator = Composer.Instance.GetPart<IDataAggregator>(); if (_dataAggregator == null) { _dataAggregator = Composer.Instance.GetExportedValueByTypeName<IDataAggregator>(Config.Get("data-aggregator", "QuantConnect.Lean.Engine.DataFeeds.AggregationManager"), forceTypeNameOnExisting: false); }

Romazes · 2025-11-18T17:29:07Z

QuantConnect.DataBento/DataBentoDataProvider.cs

-        /// <summary>
-        /// <inheritdoc cref="EventBasedDataQueueHandlerSubscriptionManager"/>
-        /// </summary>
        private EventBasedDataQueueHandlerSubscriptionManager _subscriptionManager = null!;


It is a weird code. (looks chat GPT style)
Better, you can create:

instance in global scope _subscriptionManager = new EventBasedDataQueueHandlerSubscriptionManager();

Declare the variable in the global scope and initialize it in the Initialize() method.

Please, avoid null! everywhere.

Romazes · 2025-11-18T19:46:11Z

QuantConnect.DataBento/DataBentoDataProvider.cs

-        /// </summary>
        private EventBasedDataQueueHandlerSubscriptionManager _subscriptionManager = null!;
-
        private readonly List<SubscriptionDataConfig> _activeSubscriptionConfigs = new();


We should remove the collection completely.
The Lean service EventBasedDataQueueHandlerSubscriptionManager already has helper method to get symbols.

_subscriptionManager.GetSubscribedSymbols()

spoiler: The subscription to live updates is incorrect. We should use another scheme for live updates.
https://databento.com/docs/schemas-and-data-formats/whats-a-schema#supported-schemas-and-their-fields?historical=python&live=python&reference=python

Romazes · 2025-11-18T21:41:52Z

QuantConnect.DataBento/DataBentoDataProvider.cs

                    _dataAggregator.Update(tradeBar);
-                    
+
                    Log.Trace($"DataBentoProvider.OnDataReceived(): Updated TradeBar - Symbol: {tradeBar.Symbol}, " +


You need to remove it; it's really spamming the log and making the app slower.

Romazes · 2025-11-18T21:42:10Z

QuantConnect.DataBento/DataBentoDataProvider.cs

                {
                    tradeBar.Time = GetTickTime(tradeBar.Symbol, tradeBar.Time);
                    tradeBar.EndTime = GetTickTime(tradeBar.Symbol, tradeBar.EndTime);
                    _dataAggregator.Update(tradeBar);


You should use lock(...) { ... } when calling the Update() of dataAggregator.

example

Romazes · 2025-11-18T21:44:27Z

QuantConnect.DataBento/DataBentoDataProvider.cs

            {
                if (data is Tick tick)
                {
                    tick.Time = GetTickTime(tick.Symbol, tick.Time);


The greatest way to convert time internally when creating an instance of the Tick.

Romazes · 2025-11-18T21:45:56Z

QuantConnect.DataBento/DataBentoDataProvider.cs

-                Task.Run(() =>
+
+                // Resubscribe to all active subscriptions 
+                foreach (var config in _activeSubscriptionConfigs)


Change subscription logic by using _subscriptionManager.GetSubscribedSymbols()

Romazes · 2025-11-18T21:49:22Z

QuantConnect.DataBento/DataBentoDataProvider.cs

        /// <summary>
        /// Handles connection status changes from the live client
        /// </summary>
        private void OnConnectionStatusChanged(object? sender, bool isConnected)


The method used incorrect logic and never called resubscription logic as expected.

How did you test it?

Romazes

Cool staff. Please refactor it with best practices.

Romazes · 2025-11-19T12:13:32Z

QuantConnect.DataBento/DataBentoHistoryProivder.cs

    /// </summary>
    public partial class DataBentoHistoryProvider : SynchronizingHistoryProvider
    {
        private int _dataPointCount;


Please, add xml description.

Romazes · 2025-11-19T12:14:36Z

QuantConnect.DataBento/DataBentoHistoryProivder.cs

 using QuantConnect.Lean.DataSource.DataBento;
 using QuantConnect.Interfaces;
 using System.Collections.Generic;
 using QuantConnect.Configuration;


Please remove any unused usings.

Romazes · 2025-11-19T12:22:28Z

QuantConnect.DataBento/DataBentoHistoryProivder.cs

        /// Initializes this history provider to work for the specified job
        /// </summary>
        /// <param name="parameters">The initialization parameters</param>
        public override void Initialize(HistoryProviderInitializeParameters parameters)


Hm, it is obsolete code.
Please, keep emtpy method - example
We have already initialized in DataBentoProvider.Initialize()

Romazes · 2025-11-19T12:24:07Z

QuantConnect.DataBento/DataBentoHistoryProivder.cs

    /// <summary>
    /// DataBento implementation of <see cref="IHistoryProvider"/>
    /// </summary>
    public partial class DataBentoHistoryProvider : SynchronizingHistoryProvider


It is not partial, but it needs to be partial.

Rename to DataBentoProvider

make partial main class DataBentoProvider

Romazes · 2025-11-19T12:26:23Z

QuantConnect.DataBento/DataBentoHistoryProivder.cs

    public partial class DataBentoHistoryProvider : SynchronizingHistoryProvider
    {
        private int _dataPointCount;
        private DataBentoDataDownloader _dataDownloader;


Remove it to reuse from the main class since it's partial.

Romazes · 2025-11-19T14:20:13Z

QuantConnect.DataBento/DataBentoRawLiveClient.cs

                return;
            }

            var messageCount = 0;


Why do we use it?
It is so spammy.

Recommendation: Remove it.

Romazes · 2025-11-19T14:21:58Z

QuantConnect.DataBento/DataBentoRawLiveClient.cs

                    if (line == null)
                    {
-                        Log.Trace("DatabentoRawClient.ProcessMessagesAsync(): Connection closed by server");
+                        Log.Trace("DatabentoRawClient.ProcessMessages(): Connection closed by server");


It is not a closed connection. The line is null for some reason.

Romazes · 2025-11-19T14:22:39Z

QuantConnect.DataBento/DataBentoRawLiveClient.cs

                        break;
                    }

                    if (string.IsNullOrWhiteSpace(line))


We have validated the null above.

Why do we need continue?

Use curly brackets.

Romazes · 2025-11-19T14:23:36Z

QuantConnect.DataBento/DataBentoRawLiveClient.cs

                        continue;

                    messageCount++;
                    if (messageCount <= 50 || messageCount % 100 == 0)


Please, remove it.

Romazes · 2025-11-19T14:32:12Z

QuantConnect.DataBento/DataBentoRawLiveClient.cs

                    {
                        var rtype = rtypeElement.GetInt32();

                        if (rtype == 23)


use switch/cases instead of if

Joseph-Matteo-Scorsone · 2025-12-30T00:01:32Z

In the new code I committed earlier today I kept the use of Task.Run for a message loop in the live client. I kept it because the message loop is blocking and with this block the message sequence to DataBento could become out of order and we'd never escape that blocking loop. With Task.Run the message loop can start and the Connect function can finish and return.

Improvements.

ff2ef63

jaredbroad requested changes Nov 6, 2025

View reviewed changes

Readme update, contextual log traces in for system health, not debugg…

4150782

…ing. Changed all async to sync, only using Task to connect and get messages sync.

jaredbroad approved these changes Nov 11, 2025

View reviewed changes

jaredbroad merged commit a550fb4 into QuantConnect:master Nov 11, 2025
1 check failed

Romazes reviewed Nov 17, 2025

View reviewed changes

Romazes reviewed Nov 18, 2025

View reviewed changes

Romazes reviewed Nov 19, 2025

View reviewed changes


		private readonly string _apiKey;

		private const decimal PriceScaleFactor = 1e-9m;

		@@ -57,4 +55,4 @@
		_httpClient.DefaultRequestHeaders.Authorization = new AuthenticationHeaderValue("Basic", credentials);
		}

		_dataAggregator.Update(tradeBar);


		Log.Trace($"DataBentoProvider.OnDataReceived(): Updated TradeBar - Symbol: {tradeBar.Symbol}, " +

Improvements. #3

Improvements. #3

Uh oh!

Conversation

Joseph-Matteo-Scorsone commented Nov 6, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Romazes left a comment

Choose a reason for hiding this comment

Hey, your PR needs additional improvements.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Romazes Dec 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Romazes left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Romazes left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Romazes Dec 26, 2025 •

edited

Loading