-
Notifications
You must be signed in to change notification settings - Fork 17
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
ib
rt pps update hotfix..
#350
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Not sure this didn't get caught in usage, but basically real-time updates got broken by a rework of `update_ledger_from_api_trades()`. The issue is that the ledger was being updated **before** calling `piker.pp.update_pps_conf()` which resulted in the `Position.size` not being updated correctly since the [latest added] clears passed in via the `trade_records` arg were already found in the `.clears` table and thus were causing the loop to skip the `Position.lifo_update()` call.. The solution here is to not update the ledger **until after** we call `update_pps_conf()` - it's more read/writes but it's correct and we figure out a less io heavy way to do the file writing later. Further this includes a fix to avoid double emitting a pp update caused by non-thorough logic that waits for a commission report to arrive during a fill event; previously we were emitting the same message twice due to the lack of a check for an existing comms report in the case where the report arrives *after* the fill.
goodboy
added
bug
guille broke it prolly
broker-backend
`brokerd`/`datad` related backend tech
ledger
trade, accounts and other user focal event history tracking, management and storage
labels
Jul 5, 2022
Merged
goodboy
added a commit
that referenced
this pull request
Jul 5, 2022
Addressing same issue as in #350 where we need to compute position updates using the *first read* from the ledger **before** we update it to make sure `Position.lifo_update()` gets called and **not skipped** because new trades were read as clears entries but haven't actually been included in update calcs yet.. aka we call `Position.lifo_update()`. Main change here is to convert `update_ledger()` into a context mngr so that the ledger write is committed after pps updates using `pp.update_pps_conf()`.. This is basically a hotfix to #346 as well.
goodboy
added a commit
that referenced
this pull request
Jul 6, 2022
Addressing same issue as in #350 where we need to compute position updates using the *first read* from the ledger **before** we update it to make sure `Position.lifo_update()` gets called and **not skipped** because new trades were read as clears entries but haven't actually been included in update calcs yet.. aka we call `Position.lifo_update()`. Main change here is to convert `update_ledger()` into a context mngr so that the ledger write is committed after pps updates using `pp.update_pps_conf()`.. This is basically a hotfix to #346 as well.
goodboy
added a commit
that referenced
this pull request
Jul 8, 2022
Addressing same issue as in #350 where we need to compute position updates using the *first read* from the ledger **before** we update it to make sure `Position.lifo_update()` gets called and **not skipped** because new trades were read as clears entries but haven't actually been included in update calcs yet.. aka we call `Position.lifo_update()`. Main change here is to convert `update_ledger()` into a context mngr so that the ledger write is committed after pps updates using `pp.update_pps_conf()`.. This is basically a hotfix to #346 as well.
goodboy
added a commit
that referenced
this pull request
Jul 9, 2022
Addressing same issue as in #350 where we need to compute position updates using the *first read* from the ledger **before** we update it to make sure `Position.lifo_update()` gets called and **not skipped** because new trades were read as clears entries but haven't actually been included in update calcs yet.. aka we call `Position.lifo_update()`. Main change here is to convert `update_ledger()` into a context mngr so that the ledger write is committed after pps updates using `pp.update_pps_conf()`.. This is basically a hotfix to #346 as well.
goodboy
added a commit
that referenced
this pull request
Jul 11, 2022
Addressing same issue as in #350 where we need to compute position updates using the *first read* from the ledger **before** we update it to make sure `Position.lifo_update()` gets called and **not skipped** because new trades were read as clears entries but haven't actually been included in update calcs yet.. aka we call `Position.lifo_update()`. Main change here is to convert `update_ledger()` into a context mngr so that the ledger write is committed after pps updates using `pp.update_pps_conf()`.. This is basically a hotfix to #346 as well.
goodboy
added a commit
that referenced
this pull request
Jul 14, 2022
Addressing same issue as in #350 where we need to compute position updates using the *first read* from the ledger **before** we update it to make sure `Position.lifo_update()` gets called and **not skipped** because new trades were read as clears entries but haven't actually been included in update calcs yet.. aka we call `Position.lifo_update()`. Main change here is to convert `update_ledger()` into a context mngr so that the ledger write is committed after pps updates using `pp.update_pps_conf()`.. This is basically a hotfix to #346 as well.
goodboy
added a commit
that referenced
this pull request
Jul 19, 2022
Addressing same issue as in #350 where we need to compute position updates using the *first read* from the ledger **before** we update it to make sure `Position.lifo_update()` gets called and **not skipped** because new trades were read as clears entries but haven't actually been included in update calcs yet.. aka we call `Position.lifo_update()`. Main change here is to convert `update_ledger()` into a context mngr so that the ledger write is committed after pps updates using `pp.update_pps_conf()`.. This is basically a hotfix to #346 as well.
goodboy
added a commit
that referenced
this pull request
Jul 21, 2022
Addressing same issue as in #350 where we need to compute position updates using the *first read* from the ledger **before** we update it to make sure `Position.lifo_update()` gets called and **not skipped** because new trades were read as clears entries but haven't actually been included in update calcs yet.. aka we call `Position.lifo_update()`. Main change here is to convert `update_ledger()` into a context mngr so that the ledger write is committed after pps updates using `pp.update_pps_conf()`.. This is basically a hotfix to #346 as well.
goodboy
added a commit
that referenced
this pull request
Jul 21, 2022
Addressing same issue as in #350 where we need to compute position updates using the *first read* from the ledger **before** we update it to make sure `Position.lifo_update()` gets called and **not skipped** because new trades were read as clears entries but haven't actually been included in update calcs yet.. aka we call `Position.lifo_update()`. Main change here is to convert `update_ledger()` into a context mngr so that the ledger write is committed after pps updates using `pp.update_pps_conf()`.. This is basically a hotfix to #346 as well.
goodboy
added a commit
that referenced
this pull request
Jul 21, 2022
Addressing same issue as in #350 where we need to compute position updates using the *first read* from the ledger **before** we update it to make sure `Position.lifo_update()` gets called and **not skipped** because new trades were read as clears entries but haven't actually been included in update calcs yet.. aka we call `Position.lifo_update()`. Main change here is to convert `update_ledger()` into a context mngr so that the ledger write is committed after pps updates using `pp.update_pps_conf()`.. This is basically a hotfix to #346 as well.
goodboy
added a commit
that referenced
this pull request
Jul 21, 2022
Addressing same issue as in #350 where we need to compute position updates using the *first read* from the ledger **before** we update it to make sure `Position.lifo_update()` gets called and **not skipped** because new trades were read as clears entries but haven't actually been included in update calcs yet.. aka we call `Position.lifo_update()`. Main change here is to convert `update_ledger()` into a context mngr so that the ledger write is committed after pps updates using `pp.update_pps_conf()`.. This is basically a hotfix to #346 as well.
goodboy
added a commit
that referenced
this pull request
Jul 24, 2022
Addressing same issue as in #350 where we need to compute position updates using the *first read* from the ledger **before** we update it to make sure `Position.lifo_update()` gets called and **not skipped** because new trades were read as clears entries but haven't actually been included in update calcs yet.. aka we call `Position.lifo_update()`. Main change here is to convert `update_ledger()` into a context mngr so that the ledger write is committed after pps updates using `pp.update_pps_conf()`.. This is basically a hotfix to #346 as well.
goodboy
added a commit
that referenced
this pull request
Jul 25, 2022
Addressing same issue as in #350 where we need to compute position updates using the *first read* from the ledger **before** we update it to make sure `Position.lifo_update()` gets called and **not skipped** because new trades were read as clears entries but haven't actually been included in update calcs yet.. aka we call `Position.lifo_update()`. Main change here is to convert `update_ledger()` into a context mngr so that the ledger write is committed after pps updates using `pp.update_pps_conf()`.. This is basically a hotfix to #346 as well.
goodboy
added a commit
that referenced
this pull request
Jul 26, 2022
Addressing same issue as in #350 where we need to compute position updates using the *first read* from the ledger **before** we update it to make sure `Position.lifo_update()` gets called and **not skipped** because new trades were read as clears entries but haven't actually been included in update calcs yet.. aka we call `Position.lifo_update()`. Main change here is to convert `update_ledger()` into a context mngr so that the ledger write is committed after pps updates using `pp.update_pps_conf()`.. This is basically a hotfix to #346 as well.
goodboy
added a commit
that referenced
this pull request
Jul 27, 2022
Addressing same issue as in #350 where we need to compute position updates using the *first read* from the ledger **before** we update it to make sure `Position.lifo_update()` gets called and **not skipped** because new trades were read as clears entries but haven't actually been included in update calcs yet.. aka we call `Position.lifo_update()`. Main change here is to convert `update_ledger()` into a context mngr so that the ledger write is committed after pps updates using `pp.update_pps_conf()`.. This is basically a hotfix to #346 as well.
goodboy
added a commit
that referenced
this pull request
Jul 27, 2022
Addressing same issue as in #350 where we need to compute position updates using the *first read* from the ledger **before** we update it to make sure `Position.lifo_update()` gets called and **not skipped** because new trades were read as clears entries but haven't actually been included in update calcs yet.. aka we call `Position.lifo_update()`. Main change here is to convert `update_ledger()` into a context mngr so that the ledger write is committed after pps updates using `pp.update_pps_conf()`.. This is basically a hotfix to #346 as well.
goodboy
added a commit
that referenced
this pull request
Jul 27, 2022
Addressing same issue as in #350 where we need to compute position updates using the *first read* from the ledger **before** we update it to make sure `Position.lifo_update()` gets called and **not skipped** because new trades were read as clears entries but haven't actually been included in update calcs yet.. aka we call `Position.lifo_update()`. Main change here is to convert `update_ledger()` into a context mngr so that the ledger write is committed after pps updates using `pp.update_pps_conf()`.. This is basically a hotfix to #346 as well.
goodboy
added a commit
that referenced
this pull request
Jul 27, 2022
Addressing same issue as in #350 where we need to compute position updates using the *first read* from the ledger **before** we update it to make sure `Position.lifo_update()` gets called and **not skipped** because new trades were read as clears entries but haven't actually been included in update calcs yet.. aka we call `Position.lifo_update()`. Main change here is to convert `update_ledger()` into a context mngr so that the ledger write is committed after pps updates using `pp.update_pps_conf()`.. This is basically a hotfix to #346 as well.
goodboy
added a commit
that referenced
this pull request
Jul 30, 2022
Addressing same issue as in #350 where we need to compute position updates using the *first read* from the ledger **before** we update it to make sure `Position.lifo_update()` gets called and **not skipped** because new trades were read as clears entries but haven't actually been included in update calcs yet.. aka we call `Position.lifo_update()`. Main change here is to convert `update_ledger()` into a context mngr so that the ledger write is committed after pps updates using `pp.update_pps_conf()`.. This is basically a hotfix to #346 as well.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
broker-backend
`brokerd`/`datad` related backend tech
bug
guille broke it prolly
ledger
trade, accounts and other user focal event history tracking, management and storage
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Not sure this didn't get caught in usage, but basically real-time
updates got broken by a rework of
update_ledger_from_api_trades()
.The issue is that the ledger was being updated before calling
piker.pp.update_pps_conf()
which resulted in thePosition.size
not being updated correctly since the [latest added] clears passed
in via the
trade_records
arg were already found in the.clears
tableand thus were causing the loop to skip the
Position.lifo_update()
call..
The solution here is to not update the ledger until after we call
update_pps_conf()
- it's more read/writes but it's correct and wefigure out a less io heavy way to do the file writing later.
Further this includes a fix to avoid double emitting a pp update caused
by non-thorough logic that waits for a commission report to arrive
during a fill event; previously we were emitting the same message twice
due to the lack of a check for an existing comms report in the case
where the report arrives after the fill.