rusackas commented on PR #37014: URL: https://github.com/apache/superset/pull/37014#issuecomment-4699035240
Thanks @jayakarkatika! Timezone-aware beats a fixed offset here since it handles DST, so I'm into the direction. I pushed two things to your branch: unit tests for the `normalize_dttm_col` timezone path (UTC→tz, DST, invalid-tz fallback), and a small fix - `get_dataset_timezone` read `self.extra_dict`, which lives on `SqlaTable` not the `ExploreMixin` it's defined on, so it tripped mypy and would `AttributeError` on other subclasses; reads it defensively now. Still worth a test on `get_time_filter` itself, since that's the actual boundary conversion. One design question before I'd merge though: how does the dataset-`extra` timezone play with the existing `offset`, and should this be a real dataset setting rather than a hand-edited `extra` key? Curious what you think. -- 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]
