search

mathpaquette / IQFeed.CSharpApiClient

IQFeed.CSharpApiClient is fastest and the most well-designed C# DTN IQFeed socket API connector available
MIT License
120 stars 43 forks source link

Add MarketSummaryFileFacade #135

Closed Rob-Hague closed 2 years ago

Rob-Hague commented 2 years ago

Re: #134

Can be applied to protocol_6.2 branch but used master as base since issue is not specific to 6.2 or the protocol_6.2 branch

Not sure if should close the issue since just provides a workaround

mathpaquette commented 2 years ago

@rob-hague thanks a lot! thats fine ill close once merged.

mathpaquette commented 2 years ago

@rob-hague looks like integration tests are a little bit flaky.. Not passing all the time. Did you experience that as well?

mathpaquette commented 2 years ago

image can you please have a look?

Rob-Hague commented 2 years ago

Should_Return_MarketSummaryMessages_When_GetFundamentalSummaryAsync I would expect to fail now that it is testing the right method GetEndOfDayFundamentalSummaryAsync instead of GetEndOfDaySummaryAsync and it is seeing the same exception I found in #134

The fields returned from the iqfeed api appear to be different between these requests (FDS, EDS) but unfortunately the docs don't specify what fields are returned. Thus the reason for the file facade which suits my needs

I guess a naive fix would be to add MarketSummaryFundamentalMessage following the same logic as MarketSummaryMessage but if the fields aren't documented (i.e there is no contract to what is returned) it may not be the right approach. Note that while MarketSummaryMessage parsing has some level of dyanmic-ness it still assumes all possible returned fields are contained in MarketSummaryDynamicFieldset which is the reason for the exception on FDS and the failing test