HeartSaVioR commented on issue #24996: [SPARK-28199][SS] Remove deprecated ProcessingTime URL: https://github.com/apache/spark/pull/24996#issuecomment-508312343 Let me explain my thought on the patch. Actually I assume the reason we deprecated `ProcessingTime` is that we don't want to expose implementations to user side. That is the only valid reason I can understand why they were deprecated and end users were encouraged to use static methods of Trigger instead. Not 100% sure. If that's the intention, we should hide other trigger implementations as well, so... I dug the history when it's marked as deprecated, and realize there's no explanation about the reason of deprecation. No description, no review comment on deprecating these methods. So still not sure. So based on my assumption (and my intention of the patch), it is not just renaming and changing the visibility. It finally ends up looking like that, but in fact it "removes" the deprecated class, and create the other class with applying the intention of deprecation. I will update the title of PR to include `introduce new class to replace ProcessTime`. If my assumption is incorrect, we may need to ask to ourselves why we deprecated these methods. Honestly I can't find any other reasons of doing that.
---------------------------------------------------------------- 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] With regards, Apache Git Services --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
