-
Notifications
You must be signed in to change notification settings - Fork 13k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[FLINK-16766][python]Support create StreamTableEnvironment without pa… #11769
[FLINK-16766][python]Support create StreamTableEnvironment without pa… #11769
Conversation
Thanks a lot for your contribution to the Apache Flink project. I'm the @flinkbot. I help the community Automated ChecksLast check on commit 91e1179 (Thu Apr 16 08:03:40 UTC 2020) Warnings:
Mention the bot in a comment to re-run the automated checks. Review Progress
Please see the Pull Request Review Guide for a full explanation of the review process. The Bot is tracking the review progress through labels. Labels are applied according to the order of the review items. For consensus, approval by a Flink committer of PMC member is required Bot commandsThe @flinkbot bot supports the following commands:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@SteNicholas Thanks a lot for the improvement and good job! Left some comments below.
Could you also take a look at the test failure of Travis?
>>> environment_settings = EnvironmentSettings.new_instance().use_blink_planner() \\ | ||
... .build() | ||
>>> table_env = StreamTableEnvironment.create( | ||
... env, environment_settings=environment_settings) | ||
# create with EnvironmentSettings. | ||
>>> table_env = BatchTableEnvironment.create(environment_settings=environment_settings) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
BatchTableEnvironment => StreamTableEnvironment
""" | ||
Creates a :class:`~pyflink.table.TableEnvironment` for a | ||
Creates a :class:`~pyflink.table.StreamTableEnvironment` for a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change to
Creates a :class:`~pyflink.table.StreamTableEnvironment`.
since we can also create the TableEnvironment without the execution environment.
@@ -1280,7 +1297,8 @@ def connect(self, connector_descriptor): | |||
@staticmethod | |||
def create(execution_environment=None, table_config=None, environment_settings=None): | |||
""" | |||
Creates a :class:`~pyflink.table.BatchTableEnvironment`. | |||
Creates a :class:`~pyflink.table.BatchTableEnvironment` for a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Revert the change as we can also create the TableEnvironment without the execution_environment.
…ssing StreamExecutionEnvironment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@SteNicholas Thanks a lot for the update. Looks good except for the single comment below.
@@ -296,6 +296,15 @@ def test_create_table_environment_with_blink_planner(self): | |||
planner.getClass().getName(), | |||
"org.apache.flink.table.planner.delegation.StreamPlanner") | |||
|
|||
t_env = StreamTableEnvironment.create( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be great if we can also add tests for flink planner. FYI: test will be failed for flink planner with the current PR.
…ssing StreamExecutionEnvironment
…ssing StreamExecutionEnvironment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@SteNicholas Thanks a lot for the updates. Will merge this once tests have passed.
What is the purpose of the change
This pull request will create
StreamTableEnvironment
without passingStreamExecutionEnvironment
parameter.Brief change log
StreamExecutionEnvironment
parameter to optional, support createStreamTableEnvironment
without passingStreamExecutionEnvironment
parameter through methodcreate
ofTableTableEnvironment
.Verifying this change
test_create_table_environment_with_blink_planner
method to add createStreamTableEnvironment
without passingStreamExecutionEnvironment
test case.Does this pull request potentially affect one of the following parts:
@Public(Evolving)
: (yes / no)Documentation