[GitHub] [airflow] Taragolis commented on pull request #29035: Renaming nose compatible methods in flavour of regular pytest naming
Taragolis commented on PR #29035: URL: https://github.com/apache/airflow/pull/29035#issuecomment-1400499360 > Ah - isn't that the "airflow.jobs.backfill_job.BackfillJob" generated log ? Yeah, it is `distributed.client:client.py` logger create this message ```console ERRORdistributed.client:client.py:1312 Failed to reconnect to scheduler after 30.00 seconds, closing client ``` Let's have a look if it happen more often then do something with it. Do not know about probability, maybe 1:1000 or maybe 1:1M -- 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. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] Taragolis commented on pull request #29035: Renaming nose compatible methods in flavour of regular pytest naming
Taragolis commented on PR #29035: URL: https://github.com/apache/airflow/pull/29035#issuecomment-1398885650 > No -not skipped - simply a bad timing /flaky test: > > ``` > Failed to reconnect to scheduler after 30.00 seconds, closing client > ``` Technically we should not grab this record. We grab logs and check it inside `with caplog.at_level(...)` and clear previously captured logs. So it could be: 1. A problem with `caplog` fixture, will check maybe same issue created in pytest 2. It is possible that `distributed` change some logging configuration during execution 3. Some part of Airflow logging -- 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. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] Taragolis commented on pull request #29035: Renaming nose compatible methods in flavour of regular pytest naming
Taragolis commented on PR #29035: URL: https://github.com/apache/airflow/pull/29035#issuecomment-1398808949 Just single non-relevant to this PR failure. Somehow `distributed` could avoid caplog filter: https://github.com/apache/airflow/actions/runs/3970014684/jobs/6805354398#step:6:6520 ``` with caplog.at_level(logging.ERROR, logger="airflow.jobs.backfill_job.BackfillJob"): caplog.clear() job.run() > assert "Backfill cannot be created for DagRun" in caplog.messages[0] E AssertionError: assert 'Backfill cannot be created for DagRun' in 'Failed to reconnect to scheduler after 30.00 seconds, closing client' ... -- Captured log call --- INFO airflow.models.dag:dag.py:2692 Sync 1 DAGs INFO airflow.models.dag:dag.py:2713 Creating ORM DAG for test_backfill_skip_active_scheduled_dagrun INFO airflow.models.dag:dag.py:3439 Setting next_dagrun for test_backfill_skip_active_scheduled_dagrun to 2016-01-01T00:00:00+00:00, run_after=2016-01-02T00:00:00+00:00 ERRORdistributed.client:client.py:1312 Failed to reconnect to scheduler after 30.00 seconds, closing client ERRORairflow.jobs.backfill_job.BackfillJob:backfill_job.py:830 Backfill cannot be created for DagRun scheduled__2016-01-01T00:00:00+00:00 in 2016-01-01T00:00:00, as there's already scheduled in a RUNNING state. ERRORairflow.jobs.backfill_job.BackfillJob:backfill_job.py:837 Changing DagRun into BACKFILL would cause scheduler to lose track of executing tasks. Not changing DagRun type into BACKFILL, and trying insert another DagRun into database would cause database constraint violation for dag_id + execution_date combination. Please adjust backfill dates or wait for this DagRun to finish. ``` -- 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. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] Taragolis commented on pull request #29035: Renaming nose compatible methods in flavour of regular pytest naming
Taragolis commented on PR #29035: URL: https://github.com/apache/airflow/pull/29035#issuecomment-1398705272 > > @potiuk, is it the right place? > > Yep. But you can also add "full tests needed" label on a PR to run a complete set of tests. And very easy to forget add this label even if author of the PR has access to do that. `¯\_(ツ)_/¯` This file does not change frequently. Or we could move content from pytest.ini to [pyproject.toml](https://github.com/apache/airflow/blob/main/pyproject.toml) -- 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. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] Taragolis commented on pull request #29035: Renaming nose compatible methods in flavour of regular pytest naming
Taragolis commented on PR #29035: URL: https://github.com/apache/airflow/pull/29035#issuecomment-1398319160 > Live is like a box of chocolates, you never know which one you get. You got Helm test this time :) I postpone migrate this tests to pytest as much as possible, so this looks like a right time to do this. Just need to check locally that everything work as expected and I will create PR for that. Also we need to use changes in `pytest.ini` as trigger for all tests because changes in it might affects all tests in selected directories - test - docker_tests - kubernetes_tests - dev/breeze/tests @potiuk, is it the right place? https://github.com/apache/airflow/blob/d03b9a76914babef23b84acdb6061b0b93bdc2e3/dev/breeze/src/airflow_breeze/utils/selective_checks.py#L89-L98 -- 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. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] Taragolis commented on pull request #29035: Renaming nose compatible methods in flavour of regular pytest naming
Taragolis commented on PR #29035: URL: https://github.com/apache/airflow/pull/29035#issuecomment-1398119466 Well... I thought that the k8s test failed due to the issues with Github Actions however it also use `setup` methods and we use `pytest.ini` across all tests. Let's migrate them first. -- 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. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] Taragolis commented on pull request #29035: Renaming nose compatible methods in flavour of regular pytest naming
Taragolis commented on PR #29035: URL: https://github.com/apache/airflow/pull/29035#issuecomment-1396941580 @uranusjr In additional we could add `-p no:nose` in pytest.ini adoptions, which disable autouse `setup()` and `teardown()` methods. 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. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] Taragolis commented on pull request #29035: Renaming nose compatible methods in flavour of regular pytest naming
Taragolis commented on PR #29035: URL: https://github.com/apache/airflow/pull/29035#issuecomment-1396928177 Yep, check in general this actually work (except teardown fixture) ```python import pytest class TestNoseSetupTearDown: def setup(self): self.variable = 1 def teardown(self): self.variable = 0 def test_nose_setup(self): assert self.variable == 1 class TestFixturesUsesNoseNaming: @pytest.fixture(autouse=True) def setup(self): self.variable = 1 @pytest.fixture(autouse=True) def teardown(self): """This fixture will fail until it renamed""" # No make sense to use fixture by this way, and we internally not use it yield self.variable = 0 def test_pytest_fixtures(self): assert self.variable == 1 ``` But with additional warnings ```console ❯ pytest tests/test_nose.py test session starts platform darwin -- Python 3.9.9, pytest-7.2.1, pluggy-1.0.0 rootdir: /Users/taragolis/Projects/common/airflow-local, configfile: pytest.ini plugins: subtests-0.9.0, airflow-2-0.1.0, anyio-3.6.2 collected 2 items tests/test_nose.py ..E [100%] == ERRORS === ___ ERROR at teardown of TestFixturesUsesNoseNaming.test_pytest_fixtures Fixture "teardown" called directly. Fixtures are not meant to be called directly, but are created automatically when test functions request them as parameters. See https://docs.pytest.org/en/stable/explanation/fixtures.html for more information about fixtures, and https://docs.pytest.org/en/stable/deprecations.html#calling-fixtures-directly about how to update your code. = warnings summary == tests/test_nose.py::TestNoseSetupTearDown::test_nose_setup /Users/taragolis/Projects/common/airflow-local/.nox/develop/lib/python3.9/site-packages/_pytest/fixtures.py:901: PytestRemovedIn8Warning: Support for nose tests is deprecated and will be removed in a future release. tests/test_nose.py::TestNoseSetupTearDown::test_nose_setup is using nose-specific method: `setup(self)` To remove this warning, rename it to `setup_method(self)` See docs: https://docs.pytest.org/en/stable/deprecations.html#support-for-tests-written-for-nose fixture_result = next(generator) tests/test_nose.py::TestNoseSetupTearDown::test_nose_setup /Users/taragolis/Projects/common/airflow-local/.nox/develop/lib/python3.9/site-packages/_pytest/fixtures.py:917: PytestRemovedIn8Warning: Support for nose tests is deprecated and will be removed in a future release. tests/test_nose.py::TestNoseSetupTearDown::test_nose_setup is using nose-specific method: `teardown(self)` To remove this warning, rename it to `teardown_method(self)` See docs: https://docs.pytest.org/en/stable/deprecations.html#support-for-tests-written-for-nose next(it) -- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html == short test summary info == ERROR tests/test_nose.py::TestFixturesUsesNoseNaming::test_pytest_fixtures == 2 passed, 2 warnings, 1 error in 0.03s === ``` -- 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. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] Taragolis commented on pull request #29035: Renaming nose compatible methods in flavour of regular pytest naming
Taragolis commented on PR #29035: URL: https://github.com/apache/airflow/pull/29035#issuecomment-1396909639 Well even if this work then at least this feature undocumented in `pytest`, only this methods [documented](https://docs.pytest.org/en/latest/how-to/xunit_setup.html) -- 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. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] Taragolis commented on pull request #29035: Renaming nose compatible methods in flavour of regular pytest naming
Taragolis commented on PR #29035: URL: https://github.com/apache/airflow/pull/29035#issuecomment-1396860105 And after found about deprecation functionality in [pytest's changelog](https://docs.pytest.org/en/latest/changelog.html) I finally realise why test which use setup/teardown methods work previously 藍 -- 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. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
