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

pipeline-manager: automatically queue programs for compilation #1325

Merged
merged 1 commit into from
Jan 26, 2024

Conversation

lalithsuresh
Copy link
Collaborator

@lalithsuresh lalithsuresh commented Jan 23, 2024

Both our UI and API workflows always issue a compilation request
after creating or modifying programs. There is no reason to have
progams that we don't intend on compiling.

This PR automatically sets all newly created or modified programs
to be in the "Pending" state, which automatically queues them for
compilation.

The previous /compile API is now a no-op (and shouldn't break any
existing workflows).

Feature requested by @Karakatiza666

Signed-off-by: Lalith Suresh lalith@feldera.com

Is this a user-visible change (yes/no): yes

@mihaibudiu
Copy link
Collaborator

If the user hasn't finished typing perhaps they don't want to compile yet.

@lalithsuresh
Copy link
Collaborator Author

lalithsuresh commented Jan 23, 2024

@mihaibudiu the UI in its current form doesn't know when the user will stop typing, so it always does a program modification followed by issuing a compilation request anyway. The UI also has to perform some bookkeeping to make sure the version of the program it is trying to compile is the right one.

This change allows the UI to just edit the programs and wait for the backend to update the compilation status when its done.

@Karakatiza666
Copy link
Collaborator

Karakatiza666 commented Jan 24, 2024

Actually, the UI has debouncing of the typing, so compile requests would be issued after the typing is paused for more than, say, 1 second. I think we are yet to see the UX problem with trying to compile a program half way into its body.
If we will want more manual control of when the program is "ready" to be processed - UI can just not send its code to pipeline_manager until requested.

Copy link
Contributor

@snkas snkas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it makes sense that once a program is saved, its compilation is automatically triggered. Looks good in general, one note though:

The variant ProgramStatus::None is not used anywhere anymore, although the status can exist in the database. To remove it, there would need to be a database migration that updates all "none" to "pending". If this is not done, there could be programs leftover whose compilation cannot be triggered because the endpoint is now a no-op (albeit the manual fix would be to either delete/recreate or edit them, either would result in pending status).

In future (not this PR), I'm in favor of having a button in the UI (or an option to) to save a program rather than the current auto-save when not typing briefly. Potentially also, there could be two running compiler tasks, one that compiles SQL and the other that compiles Rust, as the former is faster than the latter but is important for as fast as possible feedback.

@lalithsuresh
Copy link
Collaborator Author

@snkas yes I'm adding the migration before I remove ProgramStatus::None. Wanted to see if this passes CI first.

@lalithsuresh
Copy link
Collaborator Author

I'm also in favor of adding a button to save a program in the UI. I've experienced the compiler being triggered halfway through my typing quite often.

Signed-off-by: Lalith Suresh <lalith@feldera.com>
@lalithsuresh lalithsuresh merged commit 0e3aefa into main Jan 26, 2024
5 checks passed
@lalithsuresh lalithsuresh deleted the auto-compile branch January 26, 2024 05:48
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.

None yet

4 participants