villebro commented on PR #36368: URL: https://github.com/apache/superset/pull/36368#issuecomment-3790653115
> This is looking great! For now, I’ve just reviewed the PR description since you mentioned you’re still making changes to the code. Thanks for the initial review! > * It might be worth considering renaming the variable `is_abortable` (in the state diagram/code) to `aborting`. As it stands, having a task in progress with `is_abortable = False` could give the impression that the task cannot be aborted. If a task does not register a single `on_abort` handler, it is in fact unabortable, as it does not allow for aborting (is covered in the docs). In this case it just keeps executing until it either completes or fails (this would be the case for current thumbnail and alerts & reports tasks). In this case the cancel button is disabled, with a tooltip explaining that "This task does not support cancellation". > * I noticed you added the new tables to the PR description, which is really helpful. Could we also include a section outlining the new APIs? Good idea, I'll do that. > * You mentioned that GTF falls back to polling if a Redis operation fails. When exactly is this failed operation processed, given that polling won’t be enabled in deployments using Redis? Polling is actually the default - it will only do event based cancellation if you provide a task cache config (is covered in the docs). -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
