-
Notifications
You must be signed in to change notification settings - Fork 31
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
Simplifying DBOS TypeScript Syntax #656
Conversation
Co-authored-by: Harry Pierson <harrypierson@hotmail.com> Signed-off-by: chuck-dbos <134347445+chuck-dbos@users.noreply.github.com>
100%. The entrypoints way has huge advantages for people building new apps that want the features we started with... and there are some. There's a lot less code for something that does just scheduled workflows, say. But we will need the startup command for some of the new usecases. |
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 think this is ready to merge--the core functionality is fully working. @qianl15 should also review. Next step is templates + docs, then we can officially release it.
async function main() { | ||
await DBOS.launch({expressApp: app}); | ||
|
||
const PORT = DBOS.runtimeConfig?.port ?? 9000; |
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: The default port for Express.js is 3000. Why do we use 9000 here?
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 port does the DBOS Cloud forward?
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.
DBOS Cloud forwards to 3000 for node.js apps and 8000 for Python apps.
// DBOS uses TypeScript decorators to automatically make your functions reliable, so they need to be static. | ||
export class Hello { | ||
// Serve this function from HTTP GET requests at the /greeting endpoint with 'user' as a path parameter | ||
@DBOS.getApi("/greeting/:user") |
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.
If we want to eventually get rid of getApi
and other HTTP verbs, we should use Koa directly here.
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.
Disagree strongly.
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.
We can discuss this some other time -- maintaining a thin wrapper around Koa is pretty annoying. Remember the times we wanted to support CORS and preflight requests :/
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.
Agree with one part of the Koa point (features have to be added in the framework), strong disagree on the CORS part, baking that into your code is bad and should be managed by the deployment, which we can do better if we help you in a "serverless" way.
src/context.ts
Outdated
idAssignedForNextWorkflow?: string; | ||
queueAssignedForWorkflows?: string; | ||
parentWorkflowId?: string; | ||
parentWorkflowFid?: number; |
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 is this? This field is not used anywhere. Same as parentWorkflowId
If we can already access parentCtx, then these fields are not necessary.
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.
A general question: does it still use Stage 2 decorators?
Yeah, this is permits an incremental upgrade without having file-specific TS compiler settings. Have they made a better decorator scheme for TS? A year ago, the successor was neither better nor final. |
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.
Nice work on TS v2!
Main Change - New API
In addition to supporting all prior API constructs, DBOS Transact Typescript has a new API that does not require passing contexts around between functions. (This was inspired by the relative simplicity of the Python API.) Instead, functionality is accessed by
DBOS.<thing>
, such as@DBOS.workflow
orDBOS.knexClient
orDBOS.authenticatedUser
. You can mix old and new code.Second Big Change - Library Usage
DBOS can now be used as a library, or within Express.js or other frameworks, rather than just as serverless Koa-based entrypoints. Examples and guidance are evolving.
Other Items
Recovery has been moved to after init.