aminghadersohi commented on PR #41472:
URL: https://github.com/apache/superset/pull/41472#issuecomment-4931538167
> Type fix pushed (`7922acd3cb`). On the remaining blocker — the undelete
tools already exist as **open PR #41842** (`restore_chart` /
`restore_dashboard`): full implementations delegating to
`Restore{Chart,Dashboard}Command`, ID-or-UUID resolution, not-found /
not-in-trash / forbidden handling with the same `permission_denied` convention,
12 unit tests, CI green, and it has already been through its own review round
(LLM-context sanitization applied there too).
>
> Two ways to satisfy the blocker — happy to do either:
>
> 1. **Keep them paired as two PRs** (this one + [feat(mcp): add
restore_chart and restore_dashboard tools
#41842](https://github.com/apache/superset/pull/41842), reviewed/merged
together) — they're independent branches so either can land first; or
> 2. **Fold the restore tools into this PR** so delete+undelete ship as one
unit, closing [feat(mcp): add restore_chart and restore_dashboard tools
#41842](https://github.com/apache/superset/pull/41842).
>
> Also glad to rename `restore_*` → `undelete_*` in either case if that's
the vocabulary you want (I chose `restore_*` to match the REST route `POST
/{uuid}/restore`). Which do you prefer?
Awesome work. thank you for that. personally i am slightly in favor of 1 but
2 is also good. i'll leave it you. but i think this pr is almost set so
probably 1.
Also Superset uses "restore" everywhere - you are 100% correct; "undelete"
appears nowhere. i vote for restore_ as well.
--
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]
