-
Notifications
You must be signed in to change notification settings - Fork 72
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: add ray ids to workflows, clean up types #787
feat: add ray ids to workflows, clean up types #787
Conversation
Your org requires the Graphite merge queue for merging into mainYou must have a Graphite account and log in to Graphite in order to use the merge queue. Sign up using this link. You can enable merging using labels in your Graphite merge queue settings. |
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.
Some initial thoughts. These are more "stream of consiousness" than things that nessessarily need to be changed.
# Glossary | ||
|
||
## Worker | ||
|
||
A process that's running workflows. | ||
A process that queries for pending workflows with a specific filter. Filter is based on which workflows are registered in the given worker's registry. |
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.
This seems to imply that workflows don't need to be run on the same machine. Are
there any limitations to how this might scale?
Also, it would be cool to see a bit of a written example. Something akin to:
For example, there might be 3 workers, each with their own registry list:
Worker 1:
- cpu-intensive
- private-database
Worker 2:
- private-database
- public-database
Worker 3:
- public-database
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'm not sure what those lists are referring to in terms of an example
docs/libraries/workflow/GLOSSARY.md
Outdated
|
||
There are usually multiple workers running at the same time. | ||
A collection of registered workflows. |
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.
This makes it sound like I'd define a registry with several workflows
somewhere, then be able to re-use it in code to put several complex steps into a
single call.
It also sounds like it would be helpful to have a way to easily navigate all
existing registries in a similar way that I might go look for existing ops that do the
thing I want.
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.
then be able to re-use it in code to put several complex steps into a
single call
What do you mean by this?
Registries exist within code and do not need to be searched for anything by the developer. Its a way to tell the worker what workflows you want it to run. You define a registry in the /worker of each pkg and then in the monolith worker you import and merge them together when starting the workflow worker. They dont deal with ops at all
docs/libraries/workflow/GLOSSARY.md
Outdated
|
||
An instance of a node running a workflow. If re-running a workflow, it will be replaying events. | ||
A block of code. Can fail or not fail, used simply for tidiness. |
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'm not sure I understand what "tidiness" means in this context. I'm also
curious about the Activity vs Operation distinction. What can an Activity do
that an operation can't?
Does this basically mean that it's more like separating functions? So an example
of an Activity might be "create a new user", and this uses the operations "find
existing user" and "create new user" and "send user email"?
f9f206e
to
b6b8a23
Compare
4f009a2
to
02510dd
Compare
02510dd
to
23109a9
Compare
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.
biggest thing is: use Box<serde_json::rawvalue> everywhere (essentially a string) instead of serde_json::Value (which does a bunch of heap allocs)
b6b8a23
to
5edfdd0
Compare
23109a9
to
a9ae382
Compare
5edfdd0
to
4dd8e83
Compare
a9ae382
to
06ae309
Compare
See my comment on raw value |
4dd8e83
to
393f088
Compare
06ae309
to
6c42c6d
Compare
393f088
to
1431bf5
Compare
6c42c6d
to
33e1890
Compare
1431bf5
to
8746cdf
Compare
33e1890
to
76b70b5
Compare
8746cdf
to
3c9ae6f
Compare
76b70b5
to
f5e2153
Compare
f5e2153
to
2833a80
Compare
3c9ae6f
to
56f9056
Compare
2833a80
to
a923a66
Compare
Merge activity
|
<!-- Please make sure there is an issue that this PR is correlated to. --> ## Changes <!-- If there are frontend changes, please include screenshots. -->
56f9056
to
c90d939
Compare
a923a66
to
3072bdc
Compare
Changes