Skip to content

REFACTOR: [session] Refactor max borrowable update #1993

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Apr 23, 2025
Merged

REFACTOR: [session] Refactor max borrowable update #1993

merged 1 commit into from
Apr 23, 2025

Conversation

dboyliao
Copy link
Collaborator

No description provided.

@dboyliao dboyliao force-pushed the dboy/refactor-max-borrowable-query-worker branch 5 times, most recently from 95c284d to ac10967 Compare April 21, 2025 09:11
c9s
c9s reviewed Apr 21, 2025
View reviewed changes
c9s
c9s reviewed Apr 21, 2025
View reviewed changes
@dboyliao dboyliao force-pushed the dboy/refactor-max-borrowable-query-worker branch 2 times, most recently from 1f3ab94 to b1cf070 Compare April 21, 2025 11:04
@dboyliao dboyliao marked this pull request as ready for review April 21, 2025 11:08
@dboyliao dboyliao requested a review from c9s April 21, 2025 11:08
@dboyliao dboyliao force-pushed the dboy/refactor-max-borrowable-query-worker branch 3 times, most recently from 03dce76 to 231484b Compare April 21, 2025 16:29
c9s
c9s previously approved these changes Apr 21, 2025
View reviewed changes
@dboyliao dboyliao force-pushed the dboy/refactor-max-borrowable-query-worker branch 3 times, most recently from b8235c2 to 167b136 Compare April 22, 2025 02:53
c9s
c9s reviewed Apr 22, 2025
View reviewed changes
c9s
c9s reviewed Apr 22, 2025
View reviewed changes
pkg/bbgo/session.go Outdated
ctx context.Context,
assets ...string,
) {
if session.Margin {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you should use if !session.Margin { warn } as the function guard

c9s
c9s reviewed Apr 22, 2025
View reviewed changes
c9s
c9s reviewed Apr 22, 2025
View reviewed changes
c9s
c9s reviewed Apr 22, 2025
View reviewed changes
c9s
c9s reviewed Apr 22, 2025
View reviewed changes
c9s
c9s reviewed Apr 22, 2025
View reviewed changes
c9s
c9s reviewed Apr 22, 2025
View reviewed changes
@c9s c9s dismissed their stale review April 22, 2025 03:34

I just added new comments

c9s
c9s reviewed Apr 22, 2025
View reviewed changes
c9s
c9s reviewed Apr 22, 2025
View reviewed changes
pkg/bbgo/session.go Outdated
) {
t := time.NewTicker(timejitter.Milliseconds(interval, 500))
defer t.Stop()
defer session.logger.Debug("max borrowable updating worker exit")
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean, do we really need this log?

Copy link
Collaborator Author

@dboyliao dboyliao Apr 22, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I will remove the log.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wrap the margin update logics into a new private struct, marginInfoUpdaterType.
Conceptually, marginInfoUpdaterType is private to exchange session.
APIs of marginInfoUpdaterType are exposed via exchange session.

@dboyliao dboyliao force-pushed the dboy/refactor-max-borrowable-query-worker branch from c019795 to 5b4f4cb Compare April 22, 2025 06:07
c9s
c9s reviewed Apr 22, 2025
View reviewed changes
c9s
c9s reviewed Apr 22, 2025
View reviewed changes
c9s
c9s reviewed Apr 22, 2025
View reviewed changes
c9s
c9s reviewed Apr 22, 2025
View reviewed changes
c9s
c9s reviewed Apr 22, 2025
View reviewed changes
@dboyliao dboyliao force-pushed the dboy/refactor-max-borrowable-query-worker branch 2 times, most recently from 6dacd29 to 630c5aa Compare April 22, 2025 08:06
@dboyliao dboyliao requested a review from c9s April 22, 2025 08:06
c9s
c9s reviewed Apr 22, 2025
View reviewed changes
c9s
c9s reviewed Apr 22, 2025
View reviewed changes
c9s
c9s reviewed Apr 22, 2025
View reviewed changes
@dboyliao dboyliao force-pushed the dboy/refactor-max-borrowable-query-worker branch from 630c5aa to e88acd3 Compare April 22, 2025 08:31
@dboyliao dboyliao requested a review from c9s April 22, 2025 08:33
@dboyliao dboyliao force-pushed the dboy/refactor-max-borrowable-query-worker branch from e88acd3 to adacb6e Compare April 22, 2025 08:43
@dboyliao dboyliao force-pushed the dboy/refactor-max-borrowable-query-worker branch 5 times, most recently from 7876fa5 to e8fc767 Compare April 23, 2025 01:09
@dboyliao dboyliao force-pushed the dboy/refactor-max-borrowable-query-worker branch from e8fc767 to a328265 Compare April 23, 2025 01:29
@dboyliao dboyliao merged commit af273ba into main Apr 23, 2025
3 checks passed
@dboyliao dboyliao deleted the dboy/refactor-max-borrowable-query-worker branch April 23, 2025 01:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@c9s c9s c9s approved these changes

@ycdesu ycdesu ycdesu approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@dboyliao @c9s @ycdesu