QUIKSharp icon indicating copy to clipboard operation
QUIKSharp copied to clipboard

feature: breaking changes and new StopOrder functions

Open usikpavel opened this issue 5 years ago • 14 comments

Добрый день. У меня накопились исправления и новый функционал, которым я пользуюсь. Необходимо потестировать - либо дописать автотесты, либо погонять руками в рамках своих приложений. Я пока не смотрел, как здесь устроены автотесты, но попробую разобраться с ними и дописать, если это возможно.

Новый функционал:

StopOrderFunctions class - GetStopOrder_by_transID(), GetStopOrder_by_Number(), CreateStopOrderOrThrow(), KillStopOrderEx() methods OrderFunctions class - KillOrderEx() method StopOrder class - Datetime property

BreakingChanges:

  1. Зоопарк из reply.Comment, CLIENT_CODE и TRANS_ID (в связке TradingFunctions.SendTransaction() - QuikEvents.OnTransReplyCall()) заменил на TRANS_ID.

Пояснение.

При отладке новых методов StopOrderFunctions в коллбэке QuikEvents.OnTransReplyCall() не обнаруживалась транзакция. А мне надо положить в нее TransactionReply, чтобы корректно завершилось ожидание транзакции, например, в CreateStopOrderOrThrow(). Причина в том, что TradingFunctions.SendTransaction() сохраняет транзакцию под CLIENT_CODE (или TRANS_ID)

QuikService.Storage.Set(transaction.CLIENT_CODE, transaction);

а QuikEvents.OnTransReply() ищет ее под reply.Comment

QuikService.Storage.Get<Transaction>(reply.Comment);

reply.Comment часто приходит пустой.

Я почитал две ветки обсуждений этого зоопарка из reply.Comment, CLIENT_CODE и TRANS_ID, но так и не нашел рациональную причину его возникновения (issue #264, issue #269). Поэтому исходя из того, что QUIKSharp предназначен для использования в однопоточном режиме (т.е. требуется внешняя синхронизация как минимум для выставления транзакций), и за 1 момент времени выставляется 1 заявка, я переписал все просто на TRANS_ID. И теперь надо потестировать, у кого что сломалось. :) У меня корректно работают все используемые мною функции, связанные с TRANS_ID - отправка/отмена лимитных и рыночных ордеров, а также стоп-заявок в синхронизированном режиме (т.е. с ожиданием выполнения каждой транзакции).

Дополнение. Отправленные Transaction будут бесконечно накапливаться в QuikService.Storage. Я пока не задумывался над его очисткой, т.к. в моем случае их количество достаточно мало, чтобы влиять на память/производительность робота.

  1. В OrderFunctions в методы SendOrder(), SendMarketOrder(), SendLimitOrder() добавлен параметр clientCode

На площадке "Фондовый рынок" Московской биржи у меня всегда требуется код клиента.

usikpavel avatar Dec 16 '20 22:12 usikpavel

К сожалению, данные изменения гарантированно "сломают" работоспособность всех роботов, построенных на базе предыдущих версий библиотеки (при условии что они используют SendOrder). Такие изменения требуют тщательной подготовки, и проверки необходимости внедрения. Я как-то давно общался с @buybackoff, на тему необходимости указывать ClientCode в транзакциях, и тогда ему удалось убедить меня в отсутствии такой необходимости. Те проверки, которые я смог тогда организовать у себя, подтвердили это (речь именно про выставление заявок программным способом). Однако, я не знал (да и сейчас не знаю) что будет если в одном клиенте квика активны несколько кодов клиента (и бывает ли такое). Можете подробно описать свою ситуацию с этими Кодами клиента? Из того, что я успел по быстрому глянуть в Вашем, вроде бы все логично, и каких-то явных противоречий не вызывает. Но очень хотелось бы увидеть, что по этому поводу скажет @buybackoff.

Pr0phet1c avatar Dec 18 '20 13:12 Pr0phet1c

Мне кажется/помнится, что когда я писал эту библитеку, у меня было два разных кода клиента на основном и ФОРТСе. Какая-то заморочка с этим была. Но сейчас детали совсем не помню.

Мы не примем breaking changes. Как минимум это должно было обсуждаться изначально в issues. Всегда можно добавить overload, или функцию с суффиксом ...Ex, ...2, и т.д. если меняется поведение с теми же параметрами. Так не делается, чтобы просто взять и сломать работающий код, которым пользуются люди для торговли. Тем более, что многие (все?) клонируют репо, т.к. NuGet редко обновляется (хорошо бы сделать GitHub Action для обновления после каждого коммита).

Я поддерживаю избаление от "зоопарка", но не ломая текущий код. Можете сделать параллельную имплементацию? Или это сильно завязано на внутренности? Другой выход - добавить версию к транзакции и обрабатывать по-разному разные версии. Мы можем (аккуратно) добавлять поля к сруктурам данных, только нельзя изменять/удалять, если это не соответствует изменениям Квика.

QuikService.Storage

Я думал мы выпилили полностью storage. По крайней мере собирались это сделать, это за пределами периметра проекта. Можно удалять целиком. Кому нужно, могут сами сделать сохранение транзакций и данных любым удобным для себя способом. Кажется остался только InMemory-storage, от него все равно мало толку.

buybackoff avatar Dec 18 '20 13:12 buybackoff

@usikpavel Я сначала подумал, что это Вы с длинной историей ломающих все изменений тут: https://github.com/finsight/QUIKSharp/network

Интересно, какие планы у @Daramant на этот форк?

buybackoff avatar Dec 18 '20 13:12 buybackoff

Интересно, какие планы у @Daramant на этот форк?

Да, я тоже не так давно заметил этот форк. Там все глобально.

Pr0phet1c avatar Dec 18 '20 13:12 Pr0phet1c

Я поддерживаю избаление от "зоопарка", но не ломая текущий код. Можете сделать параллельную имплементацию? Или это сильно завязано на внутренности?

Мне кажется (могу ошибаться), можно было бы попробовать обойтись малой кровью, если реализовать что-то типа:

async Task<Order> SendOrder(string classCode, string securityCode, string accountID, Operation operation, decimal price, int qty, TransactionType orderType, string clientCode = "")

Pr0phet1c avatar Dec 18 '20 13:12 Pr0phet1c

Повторю основую философию: нужно повторить API Квик. Если я что-то изначально сделал дополнительно, то это была ошибка, нужно было делать дополнительную QuikSharp-specific API для облегчения работы с "зоопарком".

Поэтому если текущее поведение не на 100% соотвествует Квик, то стоит сделать как в Квик, и добавить наши функции, где здравого смысла usability побольше.

buybackoff avatar Dec 18 '20 18:12 buybackoff

@usikpavel Я сначала подумал, что это Вы с длинной историей ломающих все изменений тут: https://github.com/finsight/QUIKSharp/network

Интересно, какие планы у @Daramant на этот форк?

Первоначально, планы были: разобраться детально, как работает библиотека. В процессе - обратил внимание, что код местами усложнен и запутан, решил порефакторить, и что-то увлекся)) Сейчас планы: все же довести рефакторинг до конца + оптимизировать некоторые моменты работы библиотеки.

Дополнение. Отправленные Transaction будут бесконечно накапливаться в QuikService.Storage. Я пока не задумывался над его очисткой, т.к. в моем случае их количество достаточно мало, чтобы влиять на память/производительность робота.

Тоже заметил, переделаю эту часть.

Daramant avatar Dec 20 '20 19:12 Daramant

@Daramant

У Вас совсем всё breaking или public API неизменна? Планируете pool request?

Конечно в библиотекe куча технического долга. И мои знания в декабре 2014 не сравнить с текущими. Но подход был "не сломалось - не чини".

Если Вам комфортно с переделкой сложных внутренностей, могу добавить как co-maintainer сюда. Главное простое требование - не ломать публиный API.

buybackoff avatar Dec 21 '20 11:12 buybackoff

@Daramant

У Вас совсем всё breaking или public API неизменна? Планируете pool request?

В сожалению, да, много breaking changes, public api поменялось. Pool request - я бы и рад, но, т.к. много изменений, то кто такое примет)) Разве что, если вы согласны на создание QuikSharp V2.0 на основе моей ветки.

Daramant avatar Dec 21 '20 17:12 Daramant

В сожалению, да, много breaking changes, public api поменялось. Pool request - я бы и рад, но, т.к. много изменений, то кто такое примет))

Не совсем понятно, почему много breaking changes, если публичный API старается повторить API Квик. Если речь идет о других специфических для Quik# API, и Вам это интересно, то можно подумать о версии 3.0.

А можете в двух словах описать основные изменения именно функционала (кроме рефакторинга, чистки конверторов JSON.NET)? То есть что сейчас у нас не так, что может приводить к некорректной работе?

buybackoff avatar Dec 21 '20 17:12 buybackoff

А можете в двух словах описать основные изменения именно функционала (кроме рефакторинга, чистки конверторов JSON.NET)? То есть что сейчас у нас не так, что может приводить к некорректной работе?

На данный момент много изменений в плане рефакторинга: выделил интерфейсы, методы/события где-то переехали в другие классы, где-то изменились сигнатуры методов, вместо Message, ввел понятия: Command, Result, Event. Рефакторинг внутренней реализации классов. Изменения форматов данных по взаимодейтсвию с lua (например, зачем в .Net собирать строку из параметров через разделитель, а затем в lua ее обратно разбивать, если можно просто массив параметров (строк) передать в lua). До рефакторинга lua-скриптов еще не добрался.

Т.е. пока занимаюсь основным функционалом, в соответствии с целью библиотеки - пробросить функционал lua в .Net. Из дополнительного функционала рядом - будет работа с транзакциями, возможно что-то еще. Из доп. функционала были мысли: реализовать возможность фильтровать события на стороне сервера (в скрипте lua), чтобы даже не отправлять их через сокет клиенту. Т.е. в .Net отправлять только те события, на которые подписались. Но тут надо проверять/тестировать, на сколько это ускорит работу и будет ли полезно.

То есть что сейчас у нас не так, что может приводить к некорректной работе?

Просто большое количество изменений, которые необходимо проверить.

Daramant avatar Dec 21 '20 18:12 Daramant

Согласен с замечаниями, breaking changes вообще делать было стрёмно, мне тут больше нужно было ваше мнение и критика - разобраться, правильно ли я понимаю идею и текущее состояние проекта, почему в коде иногда встречаются какие-то непонятные комменты, иногда не полное единообразие в вызовах к LUA и т.д.. :) И как лучше оформлять такие изменения, и вообще нужны ли они. Пулл-реквест понадобился для демонстрации кода и запуска обсуждения (вместо issue - сразу прошу прощения, что issue сразу не завел, подумал, что в pull-request можно все обсудить и выкинуть его/допилить). Спасибо за комменты, многое вроде прояснилось, еще и форк @Daramant -а показали, посмотрю. За саму либу тоже спасибо, для меня она хорошо и понятно написана и легко поддается изменениям, а главное - РАБОТАЕТ! Намучался с StockSharp...

Может, и в самом деле пора начать новую ветку/версию - выкинуть QuikService.Storage и оставить голые привязки к LUA, а все обвесы пилить в отдельном классе/репозитории? Или взять версию @Daramant и допилить ее.

По коду, в ближайшие дни внесу изменения и создам новую ветку с non-breaking changes. (Или сюда же коммит добавить?) Только изменения многие, получается, завязаны у меня на QuikService.Storage - нужны ли они библиотеке? Могу отфильтровать и не вносить их.

usikpavel avatar Dec 22 '20 00:12 usikpavel

Забыл про clientCode написать. У меня на торговом счете (AccountNumber) на фондовом рынке есть субсчета, и к ним брокер сгенерировал разные clientCode. Вероятно, поэтому QUIK обязательно требует clientCode. Также есть просто разные торговые счета - обычный и ИИС, у них свои AccountNumber и clientCode. Может быть, с этим тоже связано.

usikpavel avatar Dec 22 '20 08:12 usikpavel

В общем, по поводу ClientCode - если реализуете то, о чем я написал чуть раньше, то думаю, что PR можно будет принять. т.к. в такой реализации ничего не должно сломаться. Если только кто-нибудь уже не нашел существенных недостатков у такого решения...

Pr0phet1c avatar Dec 22 '20 08:12 Pr0phet1c