astronomer-cosmos icon indicating copy to clipboard operation
astronomer-cosmos copied to clipboard

Testing behavior docs outdated regarding supported modes for on_warning_callback feature

Open mc51 opened this issue 10 months ago • 1 comments

The testing behavior docs are outdated. They say that only the local executor mode is supporting the on_warning_callback feature: However, it also works with the virtualenv mode. The reason for this seems to bet that virtualenv mode didn't exist when the warning feature was implemented (here's the original feature PR ).

It would be helpful to fix this in the docs: We wanted to implement this functionality and gave up after checking the docs. But after looking at the source ourselves, we figured out that it actually also works with virtualenv execution mode. We then implemented and tested the functionality successfully.

mc51 avatar Apr 09 '24 12:04 mc51

Hey @mc51,

Thanks for pointing out the discrepancy in our documentation regarding the on_warning_callback feature and its compatibility with different execution modes. You're absolutely right, and your diligence in digging into the source code has uncovered an oversight on our part.

The virtualenv execution mode indeed supports the on_warning_callback feature, and our documentation should reflect that to avoid any confusion for other users in the future. It's clear that this was an oversight from when the virtualenv mode was introduced after the original implementation of the on_warning_callback feature.

I'll make sure to update the testing behavior documentation to include virtualenv as a supported mode for the on_warning_callback feature. This update will help ensure that other users don't face the same confusion and can fully utilize the functionality without hesitation.

Your feedback is invaluable to us, and it's through contributions like yours that we can continue to improve. If you have any more insights or suggestions, please don't hesitate to share.

Thanks again for your contribution!

Best, Dosu

About Dosu

This response is meant to be useful and save you time. It is not meant to be a precise solution, but rather a starting point for your own research.

Help me be more useful! Please leave a 👍 if this is helpful and 👎 if it is irrelevant.

If you want to continue the conversation, start your reply with @dosu-bot.

dosubot[bot] avatar Apr 09 '24 12:04 dosubot[bot]

Hey @mc51 would you like to submit a PR for this?

pankajastro avatar May 17 '24 09:05 pankajastro

Looks like docs has been updated here https://astronomer.github.io/astronomer-cosmos/configuration/testing-behavior.html#warning-behavior in PR: https://github.com/astronomer/astronomer-cosmos/pull/910

pankajastro avatar Jun 03 '24 19:06 pankajastro