-
Notifications
You must be signed in to change notification settings - Fork 5.9k
chore(ext/cron): rewrite in js #32027
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
base: main
Are you sure you want to change the base?
Conversation
2cf92dd to
014cd71
Compare
4fdbc90 to
421dbe6
Compare
lucacasonato
left a comment
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.
I don't particularly like that we will now export a public span for this internal HTTP server. Can you label it in some way that makes it filterable?
36cb025 to
e16687b
Compare
lucacasonato
left a comment
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.
You removed the traceparent propagation when executing crons.
| } | ||
| } | ||
|
|
||
| setMainReadyCron(); |
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.
Crons will never register if the entrypoint never resolves, right?
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 can be registered until the entrypoint resolves, then they're locked in.
do you mean on a per-websocket-execute-message basis? |
|
Yes |
rewrite crons in js for simplicity, also rework the cron sock to be a bit easier to work with by using websockets.