-
Notifications
You must be signed in to change notification settings - Fork 978
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
graphman: create GraphQL API to execute commands #5554
Conversation
0bb243e
to
a765043
Compare
9785e67
to
6fe37ae
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.
Generally LGTM! 👍
It might be worth getting a second opinion for the command creation and extension layering APIs
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.
The code is very well written, and very clean. Nice! I am still not convinced we need all the machinery around layering and that background execution could be simplified - I'll leave a longer comment about that and some other things I noticed.
store/postgres/migrations/2024-07-23-100427_create_graphman_command_executions_table/down.sql
Outdated
Show resolved
Hide resolved
store/postgres/migrations/2024-07-23-100427_create_graphman_command_executions_table/up.sql
Outdated
Show resolved
Hide resolved
store/postgres/migrations/2024-07-23-100427_create_graphman_command_executions_table/up.sql
Outdated
Show resolved
Hide resolved
I noticed a few things in the GraphQL schema (which is well documented):
|
For the general approach how to expose the same business logic in the CLI and API, if you have a function Run
|
553cfcd
to
341fb43
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.
Very nice! I really like this and how it is put together.
1a39fe3
to
b4c7c24
Compare
This PR creates a new graphman GraphQL API with initial support for deployment info, pause, resume and restart commands. It also provides the building blocks needed to support more complex graphman commands.
Part of #3729
Note
The GraphQL API will be extended incrementally, so there will be a transition period when graphman CLI modules will have weird type conversions and potentially duplicate code. That will go away when all commands are extracted into the core crate.
Todos
Extend the API with the remaining commandsWill be in the scope of another PR