Upgrade qdownload to iqfeed 6.2

nhedlund / qdownload

IQFeed CSV market data download tool

MIT License

24 stars 14 forks source link

Upgrade qdownload to iqfeed 6.2 #7

Open mpdroog opened 5 months ago

mpdroog commented 5 months ago

Hello,

I took the liberty to upgrade your qdownload tool from iqfeed protocol 5.1 to the latest 6.2 and add some small changes I thought are useful.

Changes I've done:

(protocol) Fix intervalTypes must be lowercase
(feature) If no begin or endate set enddate=tomorrow
(feature) print config in detailed-logging
(protocol) removed requestID (field 0 is now the new messageid LM)
(protocol) added tradeaggresor/daycode/microseconds to tickdata

nhedlund commented 5 months ago

Thanks for your PR, great upgrading to the latest protocol!

I don't use IQFeed currently so I cannot test your changes unfortunately.

Can you look at the failing tests?

mpdroog commented 5 months ago

Can you look at the failing tests? Yes I'll have a look :)

mpdroog commented 5 months ago

Hi, updated the failed tests, could you approve the workflow unittest?

nhedlund commented 5 months ago

I added a couple of questions inline during my review, maybe you did not see them?

mpdroog commented 5 months ago

Aha didn't see that

mpdroog commented 5 months ago

Could you hint where I can see your inline questions? Looking at the Files changed shows no comments.

nhedlund commented 5 months ago

Wow it seems Github has issues showing inline code comments: https://github.com/orgs/community/discussions/30638

nhedlund commented 5 months ago

I am copying the comments here instead:

main.go 181 This worked before so I guess this has to do with the newer protocol?

main.go 216 Previously you did not need to set the end date, you got all data when not setting it. What happens when you don't set the end date? Can you test this?

iqfeed.go 114 If I remember correctly I used request ID to separate multiple parallel requests for different instruments. Have you tested this change on like for example 20 instruments that download in parallell and verify the data returned that the results are not scrambled (data is allocated to wrong instruments)?

mpdroog commented 5 months ago

main.go> Thank you, will try collecting the errors I got that motivated me to add this code.

iqfeed.go> I assumed the TCP-connections are separate and thus the data doesn't get scrambled. Will have to more properly test this.