YuriyKrasilnikov commented on PR #37516: URL: https://github.com/apache/superset/pull/37516#issuecomment-5063770514
> Thanks for continuing to dig on this one, @YuriyKrasilnikov. This has grown a long way past #37507 though: RLS, `security_manager.py`, the semantic layer mapper, task scheduling, and CSV escaping are all in the diff now, and none of that was asked for in the issue. That's too much surface for one PR to get a real review. > > It's also failing CI right now. `test_raise_for_access_force_dataset_match_prefers_executed_sql` and `test_update_form_data_command` are red in `security_tests.py`, and it's conflicting with `master`. > > Could we split the timing work back out to its own PR against `master`, and open separate PRs for the RLS/security and task-scheduling changes so each one gets proper eyes on it? Thanks @rusackas. This started as the timing change, but the deeper I went the more it turned out to be one system — each piece connects to the next, and things are still connecting as I work. So right now I'm continuing to build that connected system locally. What I'll come back with isn't one large diff: I'm preparing a connected graph of PRs, so each piece can be reviewed on its own and merged in a sensible order. That should make it considerably easier to review than what's in front of you now. I'll clean this PR up as well — get it back in shape, CI included. And part of what I've found falls under what's described in SECURITY.md, so that goes to [email protected] rather than into public PRs here. -- 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]
