[GitHub] agrawaldevesh commented on a change in pull request #6721: [WIP] Timestamp hackery

[GitBox <gi...@apache.org>]

agrawaldevesh commented on a change in pull request #6721: [WIP] Timestamp hackery
URL: https://github.com/apache/incubator-superset/pull/6721#discussion_r251188495
 
 

 ##########
 File path: superset/connectors/sqla/models.py
 ##########
 @@ -175,10 +175,11 @@ def dttm_sql_literal(self, dttm):
         if self.database_expression:
             return self.database_expression.format(dttm.strftime('%Y-%m-%d %H:%M:%S'))
         elif tf:
+            seconds_since_epoch = int(dttm.timestamp())
 
 Review comment:
   Actually, I think dttm.timestamp() is the right thing to do here: 
   
   The database functions epoch_to_dttm already convert the expression to the unix timestamp. So the assumption is already that epoch_s means UTC-epoch_s. So this function dttm_sql_literal converts the control input start/end timestamps to epoch-time. And I would argue that it too should behave the same way. 
   
   I can feature flag both these changes since they are "breaking" but I think the right thing to do. 
   
   I decided against introducing a new kind of epoch_s_tz because the db_engine_specs.py file already assumes that epoch_s is in UTC epoch (like it should). 
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

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