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.
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
feat: adds persistence layer for app state and job results [SBK-363] #45
feat: adds persistence layer for app state and job results [SBK-363] #45
Changes from all commits
c46ec08
b956d48
7ba2f71
85a5579
ec43642
8d2427c
2564382
e4caa59
8be2d10
82cb68e
c420f49
7e9e67a
134748d
f58911a
0506ad5
2d67e49
98a7adb
19a458b
4bb23c8
4d50d4c
2d219b1
9273cb0
f7059c4
7dc5bd0
c300fb6
e96e4f2
29fa6ce
a544e33
00b2bf9
e8dcda3
ee76273
9779c4b
0f1f7f2
8dfb17c
a0d1c20
e32297d
a7b0719
3892e13
db7cce6
60c4fd1
f35048c
40a2041
0f630b0
4a6a65a
ccc96d8
ee51bec
463f8f6
f7e44c5
4f0b11b
5bf000c
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
question(non-blocking): should we standardize
runner
toclient
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hard question, tbh. Would it be right to say "choose your client" to the user who wants to say use the WebsocketRunner?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what are the pros/cons of moving the startup/shutdown handlers into separate tasks from the worker startup/shutdown events
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They cannot be combined. They're very separate things.
Having
on_startup
be a worker event means unexpected duplication of execution. Having a separate task also allows us to feed the handler useful information, likelast_block_*
. This is basically what we need for ApePay.Renaming the previous handlers to be explicit worker event handlers implies their actual usage and is clearer, IMO. They're two distinct things.
The only real con is we're changing up behavior here. Anyone setting up worker state may need to refactor.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but won't this mean that only one worker will handle this event on startup? vs. the worker startup event is executed on every worker as it's set up?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes.
on_startup
will only run once upon runner startup.on_worker_startup
will run on every worker startup.