feat: add execute now to task queues #577
Conversation
twuebi
force-pushed
the
tp/task-cancellation
branch
3 times, most recently
from
3bfb408 to
416e76a
Compare
|
|
twuebi
force-pushed
the
tp/execute-now
branch
from
d08bc0a to
fa688c9
Compare
twuebi
changed the title
| @@ -861,6 +864,7 @@ pub mod v1 { | |||
| #[utoipa::path( | |||
| post, | |||
| tag = "warehouse", | |||
| // FIXME: deleted_tabulars should be kebab-case, this would be a breaking change | |||
There was a problem hiding this comment.
Lets document the kebab-case endpoint, and add the underscore endpoint undocumented additional for 1 minor release version.
| tag = "warehouse", | ||
| path = "/management/v1/warehouse/{warehouse_id}/deleted-tabulars/reschedule", | ||
| responses( | ||
| (status = 204, description = "Tabular dropped successfully"), |
| #[utoipa::path( | ||
| post, | ||
| tag = "warehouse", | ||
| path = "/management/v1/warehouse/{warehouse_id}/deleted-tabulars/reschedule", |
There was a problem hiding this comment.
I was thinking more about a task centric API, similar to what we discussed here:
https://docs.google.com/document/d/1sX2iBO6eL_8XCC7cznppiZWy8K44xQ-NOPhYZlHO-ic/edit?tab=t.0
But I like this approach too.
Eventually we should have a task view in Lakekeeper, where all tasks show up. This is super useful for maintenance, so that users don't have to browse all namespaces to see what ran successfully, what failed and what needs attention.
As such, we will be adding more APIs around: Retry, reschedule, cancel, listing etc.
Most of those APIs will probably use task_ids as the main identifier, which is why this API could also be useful.
I am unsure if we want to maintain both APIs.
The longer I think about it the more I think we should focus on the task centric APIs to avoid spreading task management APIs across very diverse endpoints.
Maybe we should have another call to decide if we should tackle /task now.
There was a problem hiding this comment.
I went for tabular_ids instead of task_ids to avoid messy authorization, how'd you want to deal with that?
We have per table / view permissions for drop & undrop. tasks exist on warehouse level so it'd be a warehouse permission. That'd fragment task permissions across warehouse and the entity the task pertains to, I'm somewhat ok with that but it may be unexpected to be able to undrop but not undrop-now or inversely to be able to reschedule a drop but not being able to drop in the first place.
| @@ -230,6 +230,14 @@ pub struct UndropTabularsRequest { | |||
| pub targets: Vec<TabularIdentUuid>, | |||
| } | |||
|
|
|||
| #[derive(Deserialize, Debug, ToSchema)] | |||
| #[serde(rename_all = "kebab-case")] | |||
| pub struct RescheduleSoftDeletionRequest { | |||
There was a problem hiding this comment.
If we move this to the /task API, we could use a generic reschedule request that is not specific for SoftDeletions
| .reschedule_tabular_expiration(TaskFilter::TaskIds(task_ids), reschedule_to) | ||
| .await?; | ||
|
|
||
| // TODO: emit event |
There was a problem hiding this comment.
Are we missing an event-type for this?
Being able to move the date of cleanup to now() means potentially not paying for 7 days of storage. Depending on how we wanna do recursive delete, this can also serve as a building block for it.