-
Notifications
You must be signed in to change notification settings - Fork 8
Manoj/sync invocation #1
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
Conversation
README.md
Outdated
| https://adoptium.net/en-GB/temurin/releases/?os=any&arch=any&version=21 | ||
|
|
||
| Recommended IDE IntelliJ (Community edition is fine). | ||
| But feel free to use vi, if you are more comfortable with it. |
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.
Or VS Code! :)
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.
nit: this file path name should be migrations instead of migration because we're going to have a lot more migrations in the future.
|
|
||
| CREATE INDEX workflow_status_created_at_index ON dbos.workflow_status USING btree (created_at); | ||
| CREATE INDEX workflow_status_executor_id_index ON dbos.workflow_status USING btree (executor_id); | ||
| CREATE INDEX workflow_status_status_index ON dbos.workflow_status USING btree (status); |
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.
Let's remove the tables that aren't used from this file (workflow inputs and workflow 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.
workflow_events will be needed when we do events right ? Or No ?
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.
removed workflow_inputs
| } | ||
| } | ||
|
|
||
| public static class UpdateWorkflowOptions { |
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 understand what this big class is?
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.
It is the same UpdateWorkFlowOPtions from TS and python for the updateWorkflowStatus method. The update sql is generated based on options provided.
Large ness is due to Java verbosity with get/set methods
| Logger logger = LoggerFactory.getLogger(SimpleServiceImpl.class); | ||
|
|
||
| @Workflow(name = "workWithString") | ||
| public String workWithString(String input) { |
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.
To make sure I'm clear: later on, we'll make it so workflows always return handles?
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 have thought about how it will be done. We can talk about it
Most basic:
Sync invocation of workflow
updates to workflow_status