Re: [PR] fix(dashboard): do not add importer as owner when overwriting existing dashboard [superset]

["rusackas (via GitHub)" <gi...@apache.org>]

rusackas commented on PR #38615:
URL: https://github.com/apache/superset/pull/38615#issuecomment-4318345971

   Nit, non-blocking, but would appreciate it if you could assess Copilot's point on `superset/commands/dashboard/importers/v1/utils.py:244`. `user = get_user()` is already evaluated earlier in this function for the overwrite permission checks, so the walrus rebind here is redundant. 
   
   Consider simplifying `if not existing and user and user not in dashboard.owners:` to reuse the earlier binding. Otherwise the fix looks right and the two new tests cover both branches cleanly. The failing checks (`playwright-tests (chromium)`, `codecov/project/core-packages-ts`) look unrelated to this Python-only change — worth a rebase to re-trigger.
   
   I'm willing to stamp this otherwise, but would love to see a review from @dpgaspar 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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org