You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2020/10/01 12:49:56 UTC
[GitHub] [airflow] FHoffmannCode opened a new pull request #11223: Airflow 11045 create stat name handler not supported rule
FHoffmannCode opened a new pull request #11223:
URL: https://github.com/apache/airflow/pull/11223
<!--
Thank you for contributing! Please make sure that your code changes
are covered with tests. And in case of new features or big changes
remember to adjust the documentation.
Feel free to ping committers for the review!
In case of existing issue, reference it using one of the following:
closes: #ISSUE
related: #ISSUE
How to write a good git commit message:
http://chris.beams.io/posts/git-commit/
-->
---
**^ Add meaningful description above**
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#pull-request-guidelines)** for more information.
In case of fundamental code change, Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvements+Proposals)) is needed.
In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
In case of backwards incompatible changes please leave a note in [UPDATING.md](https://github.com/apache/airflow/blob/master/UPDATING.md).
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [airflow] turbaszek commented on a change in pull request #11223: Airflow 11045 create stat name handler not supported rule
Posted by GitBox <gi...@apache.org>.
turbaszek commented on a change in pull request #11223:
URL: https://github.com/apache/airflow/pull/11223#discussion_r499419298
##########
File path: tests/upgrade/rules/test_stat_name_handler_not_supported.py
##########
@@ -0,0 +1,69 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+from unittest import mock
+
+import pytest
+from airflow.configuration import AirflowConfigParser
+
+from airflow.plugins_manager import AirflowPlugin
+from airflow.upgrade.rules.stat_name_handler_not_supported import StatNameHandlerNotSupportedRule
+from tests.plugins.test_plugin import MockPluginA, MockPluginB
+
+from tests.test_utils.mock_plugins import mock_plugin_manager
+
+
+class MockPluginWithStatNameHandler(AirflowPlugin):
+ stat_name_handler = "stat_name_handler"
+
+
+PLUGINS_WITH_STAT_NAME_HANDLER = [MockPluginWithStatNameHandler, MockPluginA]
+PLUGINS_WITHOUT_STAT_NAME_HANDLER = [MockPluginA, MockPluginB]
+CHECK_ERR_MSG = (
+ 'stat_name_handler field is no longer supported in AirflowTestPlugin.'
+ ' stat_name_handler option in [scheduler] section in airflow.cfg should'
+ ' be used to achieve the same effect.'
+)
+TEST_CONFIG = '''[scheduler]
+stat_name_handler = SCHEDULER
+'''
+
+
+class TestStatNameHandlerNotSupportedRule:
+ @pytest.mark.parametrize(
+ 'plugins, test_config, expected',
+ [
+ (PLUGINS_WITH_STAT_NAME_HANDLER, "", CHECK_ERR_MSG),
+ (PLUGINS_WITHOUT_STAT_NAME_HANDLER, "", None),
+ (PLUGINS_WITHOUT_STAT_NAME_HANDLER, TEST_CONFIG, None),
+ (PLUGINS_WITH_STAT_NAME_HANDLER, TEST_CONFIG, None),
+ ]
+ )
+ def test_check_failed(self, plugins, test_config, expected):
+ rule = StatNameHandlerNotSupportedRule()
+
+ assert isinstance(rule.description, str)
+ assert isinstance(rule.title, str)
+
+ test_conf = AirflowConfigParser()
+ test_conf.read_string(test_config)
+
+ mock_conf = mock.patch("airflow.upgrade.rules.stat_name_handler_not_supported.conf", test_conf)
Review comment:
It will be simpler to use `conf_vars`tests.tests_utils.config.contextmanger`
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #11223: Airflow 11045 create stat name handler not supported rule
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #11223:
URL: https://github.com/apache/airflow/pull/11223#issuecomment-703607628
The CI and PROD Docker Images for the build are prepared in a separate "Build Image" workflow,
that you will not see in the list of checks (you will see "Wait for images" jobs instead).
You can checks the status of those images in [The workflow run](https://github.com/apache/airflow/actions/runs/289387635)
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [airflow] FHoffmannCode commented on pull request #11223: Airflow 11045 create stat name handler not supported rule
Posted by GitBox <gi...@apache.org>.
FHoffmannCode commented on pull request #11223:
URL: https://github.com/apache/airflow/pull/11223#issuecomment-713373644
@mik-laj tests that are failing are not related to my PR, pipeline does not pass on `v1-10-test` branch.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [airflow] FHoffmannCode commented on a change in pull request #11223: Airflow 11045 create stat name handler not supported rule
Posted by GitBox <gi...@apache.org>.
FHoffmannCode commented on a change in pull request #11223:
URL: https://github.com/apache/airflow/pull/11223#discussion_r498856964
##########
File path: airflow/upgrade/rules/stat_name_handler_not_supported.py
##########
@@ -0,0 +1,38 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+from airflow.upgrade.rules.base_rule import BaseRule
+from tests.plugins.test_plugin import AirflowTestPlugin
+
+
+class StatNameHandlerNotSupportedRule(BaseRule):
+
+ title = 'stat_name_handler field in AirflowTestPlugin is not supported.'
+
+ description = '''\
+ stat_name_handler field is no longer supported in AirflowTestPlugin.
+ Instead there is stat_name_handler option in [scheduler] section of airflow.cfg.
+ This change is intended to simplify the statsd configuration.
+ '''
+
+ def check(self):
+ if getattr(AirflowTestPlugin, 'stat_name_handler'):
Review comment:
ok, fixd
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [airflow] FHoffmannCode commented on a change in pull request #11223: Airflow 11045 create stat name handler not supported rule
Posted by GitBox <gi...@apache.org>.
FHoffmannCode commented on a change in pull request #11223:
URL: https://github.com/apache/airflow/pull/11223#discussion_r499549361
##########
File path: tests/upgrade/rules/test_stat_name_handler_not_supported.py
##########
@@ -0,0 +1,69 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+from unittest import mock
+
+import pytest
+from airflow.configuration import AirflowConfigParser
+
+from airflow.plugins_manager import AirflowPlugin
+from airflow.upgrade.rules.stat_name_handler_not_supported import StatNameHandlerNotSupportedRule
+from tests.plugins.test_plugin import MockPluginA, MockPluginB
+
+from tests.test_utils.mock_plugins import mock_plugin_manager
+
+
+class MockPluginWithStatNameHandler(AirflowPlugin):
+ stat_name_handler = "stat_name_handler"
+
+
+PLUGINS_WITH_STAT_NAME_HANDLER = [MockPluginWithStatNameHandler, MockPluginA]
+PLUGINS_WITHOUT_STAT_NAME_HANDLER = [MockPluginA, MockPluginB]
+CHECK_ERR_MSG = (
+ 'stat_name_handler field is no longer supported in AirflowTestPlugin.'
+ ' stat_name_handler option in [scheduler] section in airflow.cfg should'
+ ' be used to achieve the same effect.'
+)
+TEST_CONFIG = '''[scheduler]
+stat_name_handler = SCHEDULER
+'''
+
+
+class TestStatNameHandlerNotSupportedRule:
+ @pytest.mark.parametrize(
+ 'plugins, test_config, expected',
+ [
+ (PLUGINS_WITH_STAT_NAME_HANDLER, "", CHECK_ERR_MSG),
+ (PLUGINS_WITHOUT_STAT_NAME_HANDLER, "", None),
+ (PLUGINS_WITHOUT_STAT_NAME_HANDLER, TEST_CONFIG, None),
+ (PLUGINS_WITH_STAT_NAME_HANDLER, TEST_CONFIG, None),
+ ]
+ )
+ def test_check_failed(self, plugins, test_config, expected):
+ rule = StatNameHandlerNotSupportedRule()
+
+ assert isinstance(rule.description, str)
+ assert isinstance(rule.title, str)
+
+ test_conf = AirflowConfigParser()
+ test_conf.read_string(test_config)
+
+ mock_conf = mock.patch("airflow.upgrade.rules.stat_name_handler_not_supported.conf", test_conf)
Review comment:
indeed
##########
File path: tests/upgrade/rules/test_stat_name_handler_not_supported.py
##########
@@ -0,0 +1,69 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+from unittest import mock
+
+import pytest
+from airflow.configuration import AirflowConfigParser
+
+from airflow.plugins_manager import AirflowPlugin
+from airflow.upgrade.rules.stat_name_handler_not_supported import StatNameHandlerNotSupportedRule
+from tests.plugins.test_plugin import MockPluginA, MockPluginB
+
+from tests.test_utils.mock_plugins import mock_plugin_manager
+
+
+class MockPluginWithStatNameHandler(AirflowPlugin):
+ stat_name_handler = "stat_name_handler"
+
+
+PLUGINS_WITH_STAT_NAME_HANDLER = [MockPluginWithStatNameHandler, MockPluginA]
+PLUGINS_WITHOUT_STAT_NAME_HANDLER = [MockPluginA, MockPluginB]
+CHECK_ERR_MSG = (
+ 'stat_name_handler field is no longer supported in AirflowTestPlugin.'
+ ' stat_name_handler option in [scheduler] section in airflow.cfg should'
+ ' be used to achieve the same effect.'
+)
+TEST_CONFIG = '''[scheduler]
+stat_name_handler = SCHEDULER
+'''
+
+
+class TestStatNameHandlerNotSupportedRule:
+ @pytest.mark.parametrize(
+ 'plugins, test_config, expected',
+ [
+ (PLUGINS_WITH_STAT_NAME_HANDLER, "", CHECK_ERR_MSG),
+ (PLUGINS_WITHOUT_STAT_NAME_HANDLER, "", None),
+ (PLUGINS_WITHOUT_STAT_NAME_HANDLER, TEST_CONFIG, None),
+ (PLUGINS_WITH_STAT_NAME_HANDLER, TEST_CONFIG, None),
+ ]
+ )
+ def test_check_failed(self, plugins, test_config, expected):
+ rule = StatNameHandlerNotSupportedRule()
+
+ assert isinstance(rule.description, str)
+ assert isinstance(rule.title, str)
+
+ test_conf = AirflowConfigParser()
+ test_conf.read_string(test_config)
+
+ mock_conf = mock.patch("airflow.upgrade.rules.stat_name_handler_not_supported.conf", test_conf)
Review comment:
fixd
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #11223: Airflow 11045 create stat name handler not supported rule
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #11223:
URL: https://github.com/apache/airflow/pull/11223#issuecomment-718761188
[The Workflow run](https://github.com/apache/airflow/actions/runs/335847917) is cancelling this PR. Building images for the PR has failed. Follow the the workflow link to check the reason.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [airflow] kaxil commented on pull request #11223: Create stat name handler not supported rule
Posted by GitBox <gi...@apache.org>.
kaxil commented on pull request #11223:
URL: https://github.com/apache/airflow/pull/11223#issuecomment-765711201
Can you please rebase your PR on latest v1-10-stable please, the tests were running previously -- rebasing should fix it
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #11223: Airflow 11045 create stat name handler not supported rule
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #11223:
URL: https://github.com/apache/airflow/pull/11223#issuecomment-704117308
[The Build Workflow run](https://github.com/apache/airflow/actions/runs/290977896) is cancelling this PR. The job has been cancelled by another workflow.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [airflow] turbaszek commented on pull request #11223: Airflow 11045 create stat name handler not supported rule
Posted by GitBox <gi...@apache.org>.
turbaszek commented on pull request #11223:
URL: https://github.com/apache/airflow/pull/11223#issuecomment-756834528
Rebased and I will merge once the tests pass
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [airflow] RosterIn commented on pull request #11223: Create stat name handler not supported rule
Posted by GitBox <gi...@apache.org>.
RosterIn commented on pull request #11223:
URL: https://github.com/apache/airflow/pull/11223#issuecomment-786131839
@FHoffmannCode test is failing
```
=================================== FAILURES ===================================
___________ TestAirflowMacroPluginRemovedRule.test_bad_file_failure ____________
self = <tests.upgrade.rules.test_airflow_macro_plugin_removed.TestAirflowMacroPluginRemovedRule testMethod=test_bad_file_failure>
mock_list_files = <MagicMock name='list_py_file_paths' id='140705210084688'>
def test_bad_file_failure(self, mock_list_files):
# Write a binary file
with NamedTemporaryFile("wb+", suffix=".py") as temp_file:
mock_list_files.return_value = [temp_file.name]
temp_file.write(b"{\x03\xff\x00d")
temp_file.flush()
rule = AirflowMacroPluginRemovedRule()
msgs = rule.check()
> assert 1 == len(msgs)
E AssertionError: assert 1 == 0
E + where 0 = len([])
tests/upgrade/rules/test_airflow_macro_plugin_removed.py:92: AssertionError
```
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [airflow] turbaszek commented on pull request #11223: Airflow 11045 create stat name handler not supported rule
Posted by GitBox <gi...@apache.org>.
turbaszek commented on pull request #11223:
URL: https://github.com/apache/airflow/pull/11223#issuecomment-703668057
@FHoffmannCode there's still to be unrelated commits in this PR :<
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #11223: Airflow 11045 create stat name handler not supported rule
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #11223:
URL: https://github.com/apache/airflow/pull/11223#issuecomment-703458549
The CI and PROD Docker Images for the build are prepared in a separate "Build Image" workflow,
that you will not see in the list of checks (you will see "Wait for images" jobs instead).
You can checks the status of those images in [The workflow run](https://github.com/apache/airflow/actions/runs/288946750)
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [airflow] kenluck2001 commented on a change in pull request #11223: Airflow 11045 create stat name handler not supported rule
Posted by GitBox <gi...@apache.org>.
kenluck2001 commented on a change in pull request #11223:
URL: https://github.com/apache/airflow/pull/11223#discussion_r536880288
##########
File path: airflow/upgrade/rules/stat_name_handler_not_supported.py
##########
@@ -0,0 +1,46 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+from airflow import plugins_manager
+
+from airflow.configuration import conf
+
+from airflow.upgrade.rules.base_rule import BaseRule
+
+
+class StatNameHandlerNotSupportedRule(BaseRule):
+
+ title = 'stat_name_handler field in AirflowTestPlugin is not supported.'
+
+ description = '''\
+stat_name_handler field is no longer supported in AirflowTestPlugin.
+Instead there is stat_name_handler option in [scheduler] section of airflow.cfg.
+This change is intended to simplify the statsd configuration.
+ '''
+
+ def check(self):
+ if conf.has_option('scheduler', 'stat_name_handler'):
+ return None
+
+ if any(getattr(plugin, 'stat_name_handler', None) for plugin in plugins_manager.plugins):
Review comment:
The for loop is not using plugin.The any function is not clear in its use.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [airflow] FHoffmannCode commented on pull request #11223: Airflow 11045 create stat name handler not supported rule
Posted by GitBox <gi...@apache.org>.
FHoffmannCode commented on pull request #11223:
URL: https://github.com/apache/airflow/pull/11223#issuecomment-718734208
@turbaszek @mik-laj
I've just cherry-picked my commit to `v1-10-stable` pushed the code and changed base branch to `apache/v1-10-stable`
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #11223: Create stat name handler not supported rule
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #11223:
URL: https://github.com/apache/airflow/pull/11223#issuecomment-822872615
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 5 days if no further activity occurs. Thank you for your contributions.
--
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:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on pull request #11223: Airflow 11045 create stat name handler not supported rule
Posted by GitBox <gi...@apache.org>.
mik-laj commented on pull request #11223:
URL: https://github.com/apache/airflow/pull/11223#issuecomment-712389122
> 53 successful, 40 failing, 2 cancelled, and 5 skipped checks
CI is sad. Can you fix it?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #11223: Airflow 11045 create stat name handler not supported rule
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #11223:
URL: https://github.com/apache/airflow/pull/11223#issuecomment-739364858
The PR most likely needs to run full matrix of tests because it modifies parts of the core of Airflow. However, committers might decide to merge it quickly and take the risk. If they don't merge it quickly - please rebase it to the latest master at your convenience, or amend the last commit of the PR, and push it with --force-with-lease.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [airflow] FHoffmannCode commented on a change in pull request #11223: Airflow 11045 create stat name handler not supported rule
Posted by GitBox <gi...@apache.org>.
FHoffmannCode commented on a change in pull request #11223:
URL: https://github.com/apache/airflow/pull/11223#discussion_r499560261
##########
File path: tests/test_utils/mock_plugins.py
##########
@@ -0,0 +1,67 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+from contextlib import ExitStack, contextmanager
+from unittest import mock
+
+PLUGINS_MANAGER_NULLABLE_ATTRIBUTES = [
+ "plugins",
+ "operators_modules",
+ "sensors_modules",
+ "hooks_modules",
+ "macros_modules",
+ "executors_modules",
+ "admin_views",
+ "flask_blueprints",
+ "menu_links",
+ "flask_appbuilder_views",
+ "flask_appbuilder_menu_links",
+ "global_operator_extra_links",
+ "operator_extra_links",
+ "registered_operator_link_classes",
+]
+
+
+@contextmanager
+def mock_plugin_manager(**kwargs):
+ """
+ Protects the initial state and sets the default state for the airflow.plugins module.
+ You can also overwrite variables by passing a keyword argument.
+ airflow.plugins_manager uses many global variables. To avoid side effects, this decorator performs
+ the following operations:
+ 1. saves variables state,
+ 2. set variables to default value,
+ 3. executes context code,
+ 4. restores the state of variables to the state from point 1.
+ Use this context if you want your test to not have side effects in airflow.plugins_manager, and
+ other tests do not affect the results of this test.
+ """
+ illegal_arguments = set(kwargs.keys()) - set(PLUGINS_MANAGER_NULLABLE_ATTRIBUTES) - {"import_errors"}
+ if illegal_arguments:
+ raise TypeError(
+ "TypeError: mock_plugin_manager got an unexpected keyword arguments: %s" % illegal_arguments
+ )
+ with ExitStack() as exit_stack:
+ for attr in PLUGINS_MANAGER_NULLABLE_ATTRIBUTES:
+ exit_stack.enter_context( # pylint: disable=no-member
+ mock.patch("airflow.plugins_manager.%s" % attr, kwargs.get(attr, None))
+ )
+ exit_stack.enter_context( # pylint: disable=no-member
+ mock.patch("airflow.plugins_manager.import_errors", kwargs.get("import_errors", {}))
+ )
+
+ yield
Review comment:
indeed its easier - fixd
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] closed pull request #11223: Create stat name handler not supported rule
Posted by GitBox <gi...@apache.org>.
github-actions[bot] closed pull request #11223:
URL: https://github.com/apache/airflow/pull/11223
--
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:
users@infra.apache.org
[GitHub] [airflow] mik-laj edited a comment on pull request #11223: Airflow 11045 create stat name handler not supported rule
Posted by GitBox <gi...@apache.org>.
mik-laj edited a comment on pull request #11223:
URL: https://github.com/apache/airflow/pull/11223#issuecomment-713419871
@FHoffmannCode We need to fix the v1-10-test branch before we can merge any change. Can you fix it?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [airflow] turbaszek commented on pull request #11223: Airflow 11045 create stat name handler not supported rule
Posted by GitBox <gi...@apache.org>.
turbaszek commented on pull request #11223:
URL: https://github.com/apache/airflow/pull/11223#issuecomment-706759978
@FHoffmannCode can you please rebase?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [airflow] mik-laj edited a comment on pull request #11223: Airflow 11045 create stat name handler not supported rule
Posted by GitBox <gi...@apache.org>.
mik-laj edited a comment on pull request #11223:
URL: https://github.com/apache/airflow/pull/11223#issuecomment-712389122
> 53 successful, 40 failing, 2 cancelled, and 5 skipped checks
CI is sad. Can you fix it?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [airflow] turbaszek commented on a change in pull request #11223: Airflow 11045 create stat name handler not supported rule
Posted by GitBox <gi...@apache.org>.
turbaszek commented on a change in pull request #11223:
URL: https://github.com/apache/airflow/pull/11223#discussion_r499420275
##########
File path: tests/test_utils/mock_plugins.py
##########
@@ -0,0 +1,67 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+from contextlib import ExitStack, contextmanager
+from unittest import mock
+
+PLUGINS_MANAGER_NULLABLE_ATTRIBUTES = [
+ "plugins",
+ "operators_modules",
+ "sensors_modules",
+ "hooks_modules",
+ "macros_modules",
+ "executors_modules",
+ "admin_views",
+ "flask_blueprints",
+ "menu_links",
+ "flask_appbuilder_views",
+ "flask_appbuilder_menu_links",
+ "global_operator_extra_links",
+ "operator_extra_links",
+ "registered_operator_link_classes",
+]
+
+
+@contextmanager
+def mock_plugin_manager(**kwargs):
+ """
+ Protects the initial state and sets the default state for the airflow.plugins module.
+ You can also overwrite variables by passing a keyword argument.
+ airflow.plugins_manager uses many global variables. To avoid side effects, this decorator performs
+ the following operations:
+ 1. saves variables state,
+ 2. set variables to default value,
+ 3. executes context code,
+ 4. restores the state of variables to the state from point 1.
+ Use this context if you want your test to not have side effects in airflow.plugins_manager, and
+ other tests do not affect the results of this test.
+ """
+ illegal_arguments = set(kwargs.keys()) - set(PLUGINS_MANAGER_NULLABLE_ATTRIBUTES) - {"import_errors"}
+ if illegal_arguments:
+ raise TypeError(
+ "TypeError: mock_plugin_manager got an unexpected keyword arguments: %s" % illegal_arguments
+ )
+ with ExitStack() as exit_stack:
+ for attr in PLUGINS_MANAGER_NULLABLE_ATTRIBUTES:
+ exit_stack.enter_context( # pylint: disable=no-member
+ mock.patch("airflow.plugins_manager.%s" % attr, kwargs.get(attr, None))
+ )
+ exit_stack.enter_context( # pylint: disable=no-member
+ mock.patch("airflow.plugins_manager.import_errors", kwargs.get("import_errors", {}))
+ )
+
+ yield
Review comment:
Wouldn't it be easier to just mock `airflow.upgrade.rules.stat_name_handler_not_supported.plugins_manager` and then do
```
mock_plugin_manager.plugins = [MagicMock(stat_name_handler=True)]
```
WDYT?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [airflow] turbaszek commented on pull request #11223: Airflow 11045 create stat name handler not supported rule
Posted by GitBox <gi...@apache.org>.
turbaszek commented on pull request #11223:
URL: https://github.com/apache/airflow/pull/11223#issuecomment-703081552
@FHoffmannCode can you please rebase as it seems that there were changes in the meantime?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [airflow] FHoffmannCode commented on pull request #11223: Airflow 11045 create stat name handler not supported rule
Posted by GitBox <gi...@apache.org>.
FHoffmannCode commented on pull request #11223:
URL: https://github.com/apache/airflow/pull/11223#issuecomment-703458751
@potiuk @turbaszek rebase done
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #11223: Airflow 11045 create stat name handler not supported rule
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #11223:
URL: https://github.com/apache/airflow/pull/11223#issuecomment-703164723
@FHoffmannCode Can you please rebase to latest v1-10-test ? It should work now.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [airflow] FHoffmannCode commented on a change in pull request #11223: Airflow 11045 create stat name handler not supported rule
Posted by GitBox <gi...@apache.org>.
FHoffmannCode commented on a change in pull request #11223:
URL: https://github.com/apache/airflow/pull/11223#discussion_r499549706
##########
File path: airflow/upgrade/rules/stat_name_handler_not_supported.py
##########
@@ -0,0 +1,46 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+from airflow import plugins_manager
+
+from airflow.configuration import conf
+
+from airflow.upgrade.rules.base_rule import BaseRule
+
+
+class StatNameHandlerNotSupportedRule(BaseRule):
+
+ title = 'stat_name_handler field in AirflowTestPlugin is not supported.'
+
+ description = '''\
+ stat_name_handler field is no longer supported in AirflowTestPlugin.
+ Instead there is stat_name_handler option in [scheduler] section of airflow.cfg.
+ This change is intended to simplify the statsd configuration.
Review comment:
fixd
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [airflow] FHoffmannCode commented on pull request #11223: Airflow 11045 create stat name handler not supported rule
Posted by GitBox <gi...@apache.org>.
FHoffmannCode commented on pull request #11223:
URL: https://github.com/apache/airflow/pull/11223#issuecomment-706922190
@turbaszek sure, done
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [airflow] turbaszek commented on a change in pull request #11223: Airflow 11045 create stat name handler not supported rule
Posted by GitBox <gi...@apache.org>.
turbaszek commented on a change in pull request #11223:
URL: https://github.com/apache/airflow/pull/11223#discussion_r536919803
##########
File path: airflow/upgrade/rules/stat_name_handler_not_supported.py
##########
@@ -0,0 +1,46 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+from airflow import plugins_manager
+
+from airflow.configuration import conf
+
+from airflow.upgrade.rules.base_rule import BaseRule
+
+
+class StatNameHandlerNotSupportedRule(BaseRule):
+
+ title = 'stat_name_handler field in AirflowTestPlugin is not supported.'
+
+ description = '''\
+stat_name_handler field is no longer supported in AirflowTestPlugin.
+Instead there is stat_name_handler option in [scheduler] section of airflow.cfg.
+This change is intended to simplify the statsd configuration.
+ '''
+
+ def check(self):
+ if conf.has_option('scheduler', 'stat_name_handler'):
+ return None
+
+ if any(getattr(plugin, 'stat_name_handler', None) for plugin in plugins_manager.plugins):
Review comment:
@kenluck2001 we check if any `plugin` in `plugin_manager` has `stat_name_handler` which is correct behaviour. Or do I miss something?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [airflow] turbaszek commented on a change in pull request #11223: Airflow 11045 create stat name handler not supported rule
Posted by GitBox <gi...@apache.org>.
turbaszek commented on a change in pull request #11223:
URL: https://github.com/apache/airflow/pull/11223#discussion_r499417396
##########
File path: airflow/upgrade/rules/stat_name_handler_not_supported.py
##########
@@ -0,0 +1,46 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+from airflow import plugins_manager
+
+from airflow.configuration import conf
+
+from airflow.upgrade.rules.base_rule import BaseRule
+
+
+class StatNameHandlerNotSupportedRule(BaseRule):
+
+ title = 'stat_name_handler field in AirflowTestPlugin is not supported.'
+
+ description = '''\
+ stat_name_handler field is no longer supported in AirflowTestPlugin.
+ Instead there is stat_name_handler option in [scheduler] section of airflow.cfg.
+ This change is intended to simplify the statsd configuration.
Review comment:
Please remove the indentation:
```
stat_name_handler field in AirflowTestPlugin is not supported.
--------------------------------------------------------------
stat_name_handler field is no longer supported in AirflowTestPlugin.
Instead there is stat_name_handler option in [scheduler] section of airflow.cfg.
This change is intended to simplify the statsd configuration.
```
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on a change in pull request #11223: Airflow 11045 create stat name handler not supported rule
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #11223:
URL: https://github.com/apache/airflow/pull/11223#discussion_r498363356
##########
File path: airflow/upgrade/rules/stat_name_handler_not_supported.py
##########
@@ -0,0 +1,38 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+from airflow.upgrade.rules.base_rule import BaseRule
+from tests.plugins.test_plugin import AirflowTestPlugin
+
+
+class StatNameHandlerNotSupportedRule(BaseRule):
+
+ title = 'stat_name_handler field in AirflowTestPlugin is not supported.'
+
+ description = '''\
+ stat_name_handler field is no longer supported in AirflowTestPlugin.
+ Instead there is stat_name_handler option in [scheduler] section of airflow.cfg.
+ This change is intended to simplify the statsd configuration.
+ '''
+
+ def check(self):
+ if getattr(AirflowTestPlugin, 'stat_name_handler'):
Review comment:
It is okay that there is this attribute. This allows us to create plugins that are backwards compatible.
We should check if the `stat_name_handler` option has been set. If so, skip role. If not, check to see if there are any plugins that use it. If not, skip any further processing. If so, return a message to the user.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [airflow] turbaszek commented on pull request #11223: Airflow 11045 create stat name handler not supported rule
Posted by GitBox <gi...@apache.org>.
turbaszek commented on pull request #11223:
URL: https://github.com/apache/airflow/pull/11223#issuecomment-716125300
@FHoffmannCode would you mind rebasing and changing the target base due to https://github.com/apache/airflow/pull/11719#issuecomment-714258732
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [airflow] turbaszek commented on pull request #11223: Airflow 11045 create stat name handler not supported rule
Posted by GitBox <gi...@apache.org>.
turbaszek commented on pull request #11223:
URL: https://github.com/apache/airflow/pull/11223#issuecomment-756834528
Rebased and I will merge once the tests pass
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #11223: Airflow 11045 create stat name handler not supported rule
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #11223:
URL: https://github.com/apache/airflow/pull/11223#issuecomment-703615174
[The Build Workflow run](https://github.com/apache/airflow/actions/runs/289387635) is cancelling this PR. The job has been cancelled by another workflow.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [airflow] FHoffmannCode commented on pull request #11223: Airflow 11045 create stat name handler not supported rule
Posted by GitBox <gi...@apache.org>.
FHoffmannCode commented on pull request #11223:
URL: https://github.com/apache/airflow/pull/11223#issuecomment-704089526
@turbaszek oops my bad, its fixed
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #11223: Airflow 11045 create stat name handler not supported rule
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #11223:
URL: https://github.com/apache/airflow/pull/11223#issuecomment-706938272
[The Workflow run](https://github.com/apache/airflow/actions/runs/301698429) is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks$,^Build docs$,^Spell check docs$,^Backport packages$,^Checks: Helm tests$,^Test OpenAPI*.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on pull request #11223: Airflow 11045 create stat name handler not supported rule
Posted by GitBox <gi...@apache.org>.
mik-laj commented on pull request #11223:
URL: https://github.com/apache/airflow/pull/11223#issuecomment-713419871
@FHoffmannCode We need to fix the v1-10-test branch before we can merge any change.
----------------------------------------------------------------
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:
users@infra.apache.org