eschutho commented on PR #25192: URL: https://github.com/apache/superset/pull/25192#issuecomment-1720243905
> Hey @villebro! > > Thank you for your review! > > Yes, `slice_id` is in `query_context`, however it seems that most query actions for the BaseDatasource class (which contains sql_query_mutator) uses `query_obj` as the parameter. > > We traced back to where `query_object` is [created](https://github.com/apache/superset/blob/f0c5b67515a076956771b6d6abc2e61c024bf763/superset/common/query_context_factory.py#L72) to add this `slice_id` in. Wondering if you can clarify on the harmonization part, I think given this `slice_id ` is only passed in through it's creation in `query_context_factory/query_object_factory`? > > Ah we haven't dived too much into the frontend portion of the code. Would the ask be to add something like a #slice_id into the chart editor UI so that users can easily get the ID of the chart? yeah, to me it feels a little strange that we're taking the slice_id from the form_data, and then fetching the slice from it and then getting the id off the slice, when we could just pull it from the form_data that's passed into the query_context. Which is prob another reason @villebro why you suggested to create only way to pass in the chart.. makes sense. Maybe to make this simpler for now, I'd suggest can we move forward with just using the form_data from the query_context instead of explicitly passing in the chart/slice_id into the query_obj? It also feels to me at least that the chart/slice_id has no relationship to the query object, but it does have a relationship to the form_data which is more closely related to a chart. -- 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-unsubscr...@superset.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
