Skip to content
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

SQL and serde support #1154

Draft
wants to merge 8 commits into
base: master
Choose a base branch
from
Draft

Conversation

SamuelMarks
Copy link
Contributor

WiP

Related PRs:

Took care to make the dependencies optional. I am preparing cargo-make to support #1110: asynchronous and distributed task execution. To support this, it is becoming necessary to store tasks and run task flows (execution plans). Whether that's as JSON (say for Redis or a flat-file solution), or as proper SQL tables (say for PostgreSQL or SQLite).

…`pub`lic functions necessary for other crates to implement asynchronous execution plans + execution of them
…lan is separated from run ; [src/lib/types.rs] Imbue `pub struct ExecutionPlan` with `steps_to_run` & `name` fields
Cargo.toml Outdated Show resolved Hide resolved
Cargo.toml Outdated Show resolved Hide resolved
@sagiegurari
Copy link
Owner

@SamuelMarks just released rust info and git info

@sagiegurari
Copy link
Owner

question about diesel. why do we need it integrated to cargo-make? that feels like implementation details to something you will use outside of cargo-make.
if execution plan is serializable to json, just serialize and insert that. why do i need diesel stuff all over the codebase?

@SamuelMarks
Copy link
Contributor Author

@sagiegurari It's annoying that diesel has to be depended on. AFAIK directly decorating structs of the first-party crate with derive annotations is the only way—aside from hacky nom rereads at debug-time so that a reflection style system is imposed.

Not even sure embedded types would solve this. The only obvious solution is to store struct records as JSON in a column of the SQL table. But what if I want an ExecutionPlan table? - And a Step table? - And a table joining the two that also includes State column, something like datetime stopped(Option<error>); datetime started; datetime scheduled

PS: This PR is almost ready for merge IMHO, just a little bit more rendering around the edges TODO.

Just trying to get the full demo in place:

1 . TOML → saved somewhere (e.g., flat-file JSON or database)
2. While not at end task:
a. Saved somewhere → logic to determine what step range to execute (if None then sleep and try later)
b. Execute found step range (e.g., before_each, task_name, after_each)
c. Update file || db with new status
3. Execute end task

(these steps could for example be executed in parallel and on multiple machines)

AND importantly I want to get this workflow supported without dramatically changing cargo-make. Just a little smattering of good stuff atop (error handling; use-as-library; serialising/deserialising & diesel derived types; and minor ergonomic improvements). cargo-make main use-case remains a Makefile alternative not a new Apache Airflow!

…ib/runner.rs] Make `run_protected_flow` public ; [Cargo.toml] Use official `git_info` and `rust_info` now that my PRs have been merged ;
@sagiegurari
Copy link
Owner

@SamuelMarks how about first you get some demo working for you than when you get a final understanding of what you need to make it work for you, revisit this PR and see the bare minimum needed for cargo-make to change to support it.
i prefer not to

  1. push diesel dependencies - any change i do to the structs will break peoples stuff - use json or other deserializers which could than be columnized (is it a word?)
  2. all the changes apart of that, are also kinda spooky. not sure i fully get them and why they need to be part of cargo make. for example execution plan is just a list of steps. you want also the range? have an executionplanwithstate or something and improve the runner to use that.

in general, i do'nt want to couple entire execution and structs to something very specific you are building. i want to enable extensions but not hard coupling.

@SamuelMarks SamuelMarks marked this pull request as draft August 31, 2024 20:56
@SamuelMarks
Copy link
Contributor Author

@sagiegurari Ok I've converted this to draft whilst I get it into a working state. Not long now [hopefully]!

The main issue with only supporting serde seems to be representing structs as tables in diesel. Explicitly when you add, remove, or change a struct field in cargo-make I don't want to have to update my codebase. This is explicitly coupled to your implementation.

Anyway watch this space! - Probably 24 to 48 hours away.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants