@pytest.fixture(name="caplog")
def override_caplog(request):
fixture = pytest.LogCaptureFixture(request.node)
yield fixture
fixture._finalize()
if "airflow.logging_config" in sys.modules:
import airflow.logging_config
airflow.logging_config.configure_logging()
I am 80% confident that this is going to fix the flakeyness from caplog. I’m
introducing it in my Trigger re-work PR.
> On 7 Feb 2025, at 13:17, Jarek Potiuk <ja...@potiuk.com> wrote:
>
> As clearly explained in the above example - this is not a problem with
> databricsk and azure. This is aproblem in Azure BECAUSE we move Databricks.
> So clearly a side effect of one group of tests on another. As such - we
> have no idea how many other cases we have like that and who might be
> causing it. We only know that a lot of time of people who try to keep the
> main green is lost because of that.
>
>> This is not very collaborative and accepting that people have a life
> outside of their computer’s Jarek.
>
> No. It's collaborative. This is how our social contract works. If you want
> to change the rules and have good reasoning for that - propose a change.
> And you still can propose to revert the decision in a separate thread. This
> happened in the past and it's perfectly fine.
>
> We cannot completely change the social agreement we have on decision making
> work in our community because one person missed a thread on a really
> insignificant thing. We should just accept it and follow, or propose to
> change a contract. That's how a community works.
>
>
> J
>
>
>
> On Fri, Feb 7, 2025 at 2:05 PM Jarek Potiuk <ja...@potiuk.com> wrote:
>
>>> How is it flakey though? This is the part I’m asking for explaining
>> about. Surely it’s messed up and _ALWAYS_ messed up?
>>
>> See above example posted in the thread. It explains everything. I am not
>> sure if you missed it, but it's the third time I am explaining it and you
>> keep on asking. The example I have shows not only the copy & paste of the
>> problem, but link to the exact failure.
>>
>> On Fri, Feb 7, 2025 at 2:00 PM Ash Berlin-Taylor <a...@apache.org> wrote:
>>
>>> How does caplog introduce flakiness though? That still isn’t
>>>
>>>
>>>> On 7 Feb 2025, at 11:07, Jarek Potiuk <ja...@potiuk.com> wrote:
>>>>
>>>> Ash - take a close look at the example I sent. Maybe you missed it.
>>>>
>>>> * Caplog gets logs from loggers - If your loggers are messed 1000 tests
>>>> before and not restored - you will have failing tests. Side effects
>>>
>>> How is it flakey though? This is the part I’m asking for explaining
>>> about. Surely it’s messed up and _ALWAYS_ messed up?
>>>
>>>>
>>>> * Mocking logs is mocking lag method call. It always work and is
>>>> side-effect resistant
>>>>
>>>>
>>>> I would only agree to restore back Caplog usag (which as I mentioned
>>>> requires new proposal, new consensus or voting as the consensus is
>>> already
>>>> reached). I will expect the person proposing it to track all the cases
>>>> where side effects have been causing problems
>>>
>>> This is not very collaborative and accepting that people have a life
>>> outside of their computer’s Jarek.
>>>
>>>
>>> Here’s a proposal for you: don’t create needless code churn and wholesale
>>> replace caplog, instead only swap if it’s not actually a problem. Replace
>>> it only when it is noticed as being flakey - which you’ve pointed out two
>>> cases, Databricks and Azure.. We have 287 instances of `caplog` in our
>>> tests. How many are actually flakey? Why change things that aren’t broken?
>>>
>>>>
>>>>
>>>> This happened (and let me repeat it again - I hope it will not be again
>>>> ignored this time). About 10 to 15 times in the last few weeks.
>>>>
>>>> Ash - if you want to propose restoring Caplog usage, please start a new
>>>> thread. Ideally with explanation why.
>>>>
>>>> J.
>>>>
>>>> pt., 7 lut 2025, 10:15 użytkownik Ash Berlin-Taylor <a...@apache.org>
>>>> napisał:
>>>>
>>>>> How does replacing caplog with mocking help anything, assuming that you
>>>>> test what is logged against the mock?
>>>>>
>>>>> Please explain it to me like I’m 5, cos I don’t see how it makes the
>>>>> blindest bit of difference. Testing what is on the caplog output vs
>>> what is
>>>>> recorded on the mock log object is functional identical. If caplog
>>> tests
>>>>> are flakey then mock log tests will be equally flakey. The caplog
>>> process
>>>>> itself is not flakey. It’s what is producing the log that is flakey,
>>> no?
>>>>>
>>>>> If you are talking about removing logging tests entirely if they aren’t
>>>>> providing value: sure, but that isn’t what I understood this to be
>>> about.
>>>>>
>>>>>> On 6 Feb 2025, at 19:41, Jarek Potiuk <ja...@potiuk.com> wrote:
>>>>>>
>>>>>> BTW. Ash - good example of why we want to get rid of caplog is here:
>>>>>>
>>>>>>
>>>>>
>>> https://github.com/apache/airflow/actions/runs/13185566641/job/36807274106
>>>>>>
>>>>>> This is PR to move databricks provider to the new provider structure.
>>>>>> Suddenly in that PR ***WASB*** (yes - we moved **DATABRICKS**) tests
>>> are
>>>>>> failing:
>>>>>>
>>>>>>
>>>>>> =================================== FAILURES
>>>>>> ===================================
>>>>>> 2559
>>>>>> <
>>>>>
>>> https://github.com/apache/airflow/actions/runs/13185566641/job/36807274106#step:6:2576
>>>>>> _______
>>>>>> TestWasbHook.test_delete_container_generic_exception[ValueError]
>>> _______
>>>>>> 2560
>>>>>> <
>>>>>
>>> https://github.com/apache/airflow/actions/runs/13185566641/job/36807274106#step:6:2577
>>>>>>
>>>>>> providers/tests/microsoft/azure/hooks/test_wasb.py:539: in
>>>>>> test_delete_container_generic_exception
>>>>>> 2561
>>>>>> <
>>>>>
>>> https://github.com/apache/airflow/actions/runs/13185566641/job/36807274106#step:6:2578
>>>>>>
>>>>>> assert "Error deleting container: mycontainer" in caplog.text
>>>>>> 2562
>>>>>> <
>>>>>
>>> https://github.com/apache/airflow/actions/runs/13185566641/job/36807274106#step:6:2579
>>>>>> E
>>>>>> AssertionError: assert 'Error deleting container: mycontainer' in ''
>>>>>> 2563
>>>>>> <
>>>>>
>>> https://github.com/apache/airflow/actions/runs/13185566641/job/36807274106#step:6:2580
>>>>>> E
>>>>>> + where '' = <_pytest.logging.LogCaptureFixture object at
>>>>>> 0x7fa7109dcbe0>.text
>>>>>> 2564
>>>>>> <
>>>>>
>>> https://github.com/apache/airflow/actions/runs/13185566641/job/36807274106#step:6:2581
>>>>>> ______
>>>>>> TestWasbHook.test_delete_container_generic_exception[RuntimeError]
>>> ______
>>>>>> 2565
>>>>>> <
>>>>>
>>> https://github.com/apache/airflow/actions/runs/13185566641/job/36807274106#step:6:2582
>>>>>>
>>>>>> providers/tests/microsoft/azure/hooks/test_wasb.py:539: in
>>>>>> test_delete_container_generic_exception
>>>>>> 2566
>>>>>> <
>>>>>
>>> https://github.com/apache/airflow/actions/runs/13185566641/job/36807274106#step:6:2583
>>>>>>
>>>>>> assert "Error deleting container: mycontainer" in caplog.text
>>>>>> 2567
>>>>>> <
>>>>>
>>> https://github.com/apache/airflow/actions/runs/13185566641/job/36807274106#step:6:2584
>>>>>> E
>>>>>> AssertionError: assert 'Error deleting container: mycontainer' in ''
>>>>>> 2568
>>>>>> <
>>>>>
>>> https://github.com/apache/airflow/actions/runs/13185566641/job/36807274106#step:6:2585
>>>>>> E
>>>>>> + where '' = <_pytest.logging.LogCaptureFixture object at
>>>>>> 0x7fa710a65040>.text
>>>>>> 2569
>>>>>> <
>>>>>
>>> https://github.com/apache/airflow/actions/runs/13185566641/job/36807274106#step:6:2586
>>>>>> ____________
>>>>>> TestWasbHook.test_delete_container_resource_not_found _____________
>>>>>> 2570
>>>>>> <
>>>>>
>>> https://github.com/apache/airflow/actions/runs/13185566641/job/36807274106#step:6:2587
>>>>>>
>>>>>> providers/tests/microsoft/azure/hooks/test_wasb.py:548: in
>>>>>> test_delete_container_resource_not_found
>>>>>> 2571
>>>>>> <
>>>>>
>>> https://github.com/apache/airflow/actions/runs/13185566641/job/36807274106#step:6:2588
>>>>>>
>>>>>> assert "Unable to delete container mycontainer (not found)" in
>>>>> caplog.text
>>>>>> 2572
>>>>>> <
>>>>>
>>> https://github.com/apache/airflow/actions/runs/13185566641/job/36807274106#step:6:2589
>>>>>> E
>>>>>> AssertionError: assert 'Unable to delete container mycontainer (not
>>>>> found)'
>>>>>> in ''
>>>>>> 2573
>>>>>> <
>>>>>
>>> https://github.com/apache/airflow/actions/runs/13185566641/job/36807274106#step:6:2590
>>>>>> E
>>>>>> + where '' = <_pytest.logging.LogCaptureFixture object at
>>>>>> 0x7fa710c37bb0>.text
>>>>>>
>>>>>>
>>>>>> And those are the failing tests. They are using caplog and I am not
>>> sure
>>>>> if
>>>>>> you can figure out why suddenly the expected log entries cannot be
>>> found
>>>>> in
>>>>>> caplog.
>>>>>>
>>>>>> My best guess is that somewhere, some tests (databricks) are
>>>>> reconfiguring
>>>>>> the loggers, and it messes up with caplog. Classic side effects.
>>>>>>
>>>>>> Mocking log methods directly avoids side-effects involved with logger
>>>>>> reconfiguraton. This is about 10th time we have this problem while
>>>>> moving
>>>>>> providers.
>>>>>> Here are the tests that are failing.
>>>>>>
>>>>>> @pytest.mark.parametrize("exc", [ValueError, RuntimeError])
>>>>>> def test_delete_container_generic_exception(self, exc:
>>>>> type[Exception],
>>>>>> caplog):
>>>>>> hook = WasbHook(wasb_conn_id=self.azure_shared_key_test)
>>>>>> with mock.patch.object(WasbHook, "_get_container_client") as m:
>>>>>> m.return_value.delete_container.side_effect =
>>>>>> exc("FakeException")
>>>>>> caplog.clear()
>>>>>> caplog.set_level("ERROR")
>>>>>> with pytest.raises(exc, match="FakeException"):
>>>>>> hook.delete_container("mycontainer")
>>>>>> assert "Error deleting container: mycontainer" in
>>> caplog.text
>>>>>>
>>>>>> def test_delete_container_resource_not_found(self, caplog):
>>>>>> hook = WasbHook(wasb_conn_id=self.azure_shared_key_test)
>>>>>> with mock.patch.object(WasbHook, "_get_container_client") as m:
>>>>>> m.return_value.delete_container.side_effect =
>>>>>> ResourceNotFoundError("FakeException")
>>>>>> caplog.clear()
>>>>>> caplog.set_level("WARNING")
>>>>>> hook.delete_container("mycontainer")
>>>>>> assert "Unable to delete container mycontainer (not found)"
>>> in
>>>>>> caplog.text
>>>>>>
>>>>>> @mock.patch.object(WasbHook, "delete_blobs")
>>>>>> def test_delete_single_blob(self, delete_blobs,
>>>>>> mocked_blob_service_client):
>>>>>> hook = WasbHook(wasb_conn_id=self.azure_shared_key_test)
>>>>>> hook.delete_file("container", "blob", is_prefix=False)
>>>>>> delete_blobs.assert_called_once_with("container", "blob")
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> On Thu, Feb 6, 2025 at 3:56 PM Jarek Potiuk <ja...@potiuk.com> wrote:
>>>>>>
>>>>>>> Just - note that the 72 hrs are specifically designed to cover the
>>>>>>> weekend. So when there are such - rather insignificant - issues, I
>>>>> think we
>>>>>>> should be ok. We have had many of our votes - including the
>>> absolutely
>>>>> most
>>>>>>> important - releases - lasting 72 hours over weekend and it has never
>>>>> been
>>>>>>> a problem, we only extended it including weekends where the matters
>>> were
>>>>>>> really important and we expected them to be contentious (this one was
>>>>> not).
>>>>>>>
>>>>>>> Does it mean Ash that we propose extending all kinds of consensus and
>>>>>>> voting in this way ? It's the first time it has been raised, so maybe
>>>>>>> that's a good idea to make it longer now when we are all additionally
>>>>> busy
>>>>>>> with Airflow 3, but I don't see any reason why it should be a problem
>>>>> "in
>>>>>>> general". WDYT?
>>>>>>>
>>>>>>> Note that your objection is also 5 days after the initial message, so
>>>>> even
>>>>>>> if it would be 2 days more, that would not change anything.
>>>>>>>
>>>>>>> IMHO the consensus is reached. and If you want to revert that you
>>> should
>>>>>>> start a new thread Ash.
>>>>>>>
>>>>>>> J.
>>>>>>>
>>>>>>> On Thu, Feb 6, 2025 at 3:35 PM Ash Berlin-Taylor <a...@apache.org>
>>>>> wrote:
>>>>>>>
>>>>>>>> Please be aware of time when you are proposing this. 48 out of the
>>> 72
>>>>>>>> hours your had here were over the weekend which doesn’t give many
>>>>> people
>>>>>>>> adequate time to read and think upon this.
>>>>>>>>
>>>>>>>>
>>>>>>>> Because I’m -1 on this as I don’t have enough info (see my other
>>> email)
>>>>>>>> to understand why this change makes the first bit of difference.
>>>>>>>>
>>>>>>>>> On 4 Feb 2025, at 22:38, Buğra Öztürk <ozturkbugr...@gmail.com>
>>>>> wrote:
>>>>>>>>>
>>>>>>>>> Hey all,
>>>>>>>>>
>>>>>>>>> The consensus has been reached.
>>>>>>>>>
>>>>>>>>> Regards,
>>>>>>>>>
>>>>>>>>> On Sat, Feb 1, 2025 at 5:38 PM Buğra Öztürk <
>>> ozturkbugr...@gmail.com>
>>>>>>>> wrote:
>>>>>>>>>
>>>>>>>>>> Hello everyone,
>>>>>>>>>>
>>>>>>>>>> There have been recurring discussions about minimizing the usage
>>> of
>>>>>>>> caplog
>>>>>>>>>> in unit tests. If logs are necessary for a test, we should mock
>>> them
>>>>>>>> unless
>>>>>>>>>> the test strictly requires actual log output. Otherwise, the
>>> caplog
>>>>>>>> tests
>>>>>>>>>> should be removed and logs should be mocked.
>>>>>>>>>>
>>>>>>>>>> During recent large migrations, changes and ongoing CI/CD efforts,
>>>>>>>> caplog
>>>>>>>>>> has proven to be flaky in unit tests. They frequently cause red
>>>>>>>> pipelines
>>>>>>>>>> in PRs and scheduled CI/CD runs. There have been local
>>> discussions on
>>>>>>>>>> removing caplog from unit tests. To formalize this, I propose a
>>> lazy
>>>>>>>>>> consensus to remove and prevent caplog usage in unit tests,
>>> ensuring
>>>>>>>> logs
>>>>>>>>>> are mocked when needed and disallowing caplog without mocking
>>> unless
>>>>>>>>>> explicitly approved.
>>>>>>>>>>
>>>>>>>>>> *Why should caplog be avoided?*
>>>>>>>>>>
>>>>>>>>>> - Big maintenance effort on CI/CD
>>>>>>>>>> - Instability
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> *What should we do instead if we use caplog in a unit test?*
>>>>>>>>>>
>>>>>>>>>> - Mocking the Log
>>>>>>>>>>
>>>>>>>>>> *In the Scope of this consensus*
>>>>>>>>>>
>>>>>>>>>> - Remove caplog usage from unit tests if possible
>>>>>>>>>> - Mock logs and remove caplog from unit tests if possible
>>>>>>>>>> - Exceptional cases will be subject to maintainer approval
>>>>>>>>>> - Prevent caplog to be included in unit tests without explicitly
>>>>>>>>>> mocking the log
>>>>>>>>>>
>>>>>>>>>> *Action Items related to the above Scope:*
>>>>>>>>>>
>>>>>>>>>> - Scan and replace caplog tests with mocking where possible
>>>>>>>>>> - Scan and remove caplog tests where possible
>>>>>>>>>> - Include CI check to prevent adding additional caplog tests
>>> without
>>>>>>>>>> Mocking and/or without approval from a maintainer to allow
>>>>>>>> flexibility in
>>>>>>>>>> some exceptional cases
>>>>>>>>>> - Create documentation with examples for implementing tests using
>>>>>>>> logs
>>>>>>>>>> without caplog (e.g., using mocking or avoiding logs entirely)
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> The lazy consensus will be reached on 2025-02-04, (unless someone
>>>>>>>> surfaces
>>>>>>>>>> an objection).
>>>>>>>>>> --
>>>>>>>>>> Bugra Ozturk
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> --
>>>>>>>>> Bugra Ozturk
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>> ---------------------------------------------------------------------
>>>>>>>> To unsubscribe, e-mail: dev-unsubscr...@airflow.apache.org
>>>>>>>> For additional commands, e-mail: dev-h...@airflow.apache.org
>>>>>>>>
>>>>>>>>
>>>>>
>>>>>
>>>
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: dev-unsubscr...@airflow.apache.org
>>> For additional commands, e-mail: dev-h...@airflow.apache.org
>>>
>>>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@airflow.apache.org
For additional commands, e-mail: dev-h...@airflow.apache.org
