zhaoyongjie commented on pull request #12552:
URL: https://github.com/apache/superset/pull/12552#issuecomment-762867046


   > I agree with the change, but this change will break existing charts 
relying on the new logic. This is a chart that was working before but now is 
failing with the new validation:
   > 
![image](https://user-images.githubusercontent.com/33317356/104884789-9cb0b380-596f-11eb-9b4f-c413d34570a7.png)
   > Since it seems `dateutil.parser.parse` interprets "7 days" as 
interchangeable with "7 days ago", we should perhaps do a migration that adds 
the "ago" suffix to old time ranges to avoid breaking them? Alternatively we 
should potentially only do the validation for new charts, and let old charts 
keep working with the deprecated ranges.
   
   I'm concerned that a db metadata with a large number of charts will require 
some migration risk to do this migration.
   
   Are we considering compatibility with the old syntax?  I would prefer to be 
compatible with the original syntax.
   
   What do you think about this? @ktmud @villebro 


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to