[
https://issues.apache.org/jira/browse/FLINK-34527?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18111143#comment-18111143
]
Bhanu Chander Vallabaneni commented on FLINK-34527:
---------------------------------------------------
I would like to pick this up — could a committer assign it to me?
Scoping it first, because the PyFlink side is larger than the Java change was
and I would rather
agree the shape before writing it.
What is already in place: PyFlink has both {{Time}} and {{Duration}} in
{{pyflink/common/time.py}}, and every API that takes a {{Time}} converts it
through
{{Time.to_milliseconds()}} before crossing to the JVM — 15 call sites, in
{{pyflink/datastream/window.py}}, {{async_data_stream.py}} and {{state.py}}. So
the mechanics of
accepting something else are simple.
What makes it more than a decorator: {{Time.<factory>}} is used *32 times
inside pyflink itself*,
outside tests, plus 31 times in its own tests. Deprecating the class without
touching those means
Flink's own code and test suite start emitting deprecation warnings, and users
of the window,
async-I/O and state-TTL APIs would get a warning with no alternative to move
to, since those APIs
accept only {{Time}} today.
So I propose:
1. accept {{Duration}} (and, for a Pythonic option, {{datetime.timedelta}})
everywhere a {{Time}} is
accepted today, normalising through one small helper — no behaviour change
for existing callers;
2. mark {{Time}} and its factory methods with the existing {{@Deprecated}}
decorator from
{{pyflink/util/api_stability_decorators.py}}, pointing at {{Duration}};
3. migrate pyflink's own 32 internal usages so the project does not warn on
itself;
4. check the docs code samples, as you noted.
One question before I start: for the public parameter type, do you want
{{Duration}} only — mirroring
{{java.time.Duration}} from FLINK-32570 — or should {{datetime.timedelta}} be
accepted as well? I
lean towards accepting both and documenting {{Duration}} as the canonical one,
but that is an API
decision and I would rather follow your preference than guess.
Happy to split it as (1)+(2) first and (3)+(4) as a follow-up if you would
prefer smaller reviews.
> Deprecate Time classes also in PyFlink
> --------------------------------------
>
> Key: FLINK-34527
> URL: https://issues.apache.org/jira/browse/FLINK-34527
> Project: Flink
> Issue Type: Bug
> Components: API / Python
> Affects Versions: 1.20.0
> Reporter: Matthias Pohl
> Priority: Major
>
> FLINK-32570 deprecated the Time classes. But we missed touched the
> PyFlink-related APIs. Also the Docs should be checked for code samples
> related to Time classes.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
