Re: [PR] fix: remove trigger event payload from INFO log in run_trigger() [airflow]
Feudjo commented on PR #67244: URL: https://github.com/apache/airflow/pull/67244#issuecomment-4827244108 Hi @potiuk, All CI checks are passing. Could you please take another look when you have a moment? Thank you! -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] fix: remove trigger event payload from INFO log in run_trigger() [airflow]
Feudjo commented on PR #67244: URL: https://github.com/apache/airflow/pull/67244#issuecomment-4714901475 Hi @potiuk I'm seeing failures in airflow_local_settings.py in the CI run, but this file isn't part of my changes. I tried reproducing locally and I'm not sure if this is pre-existing on main or something my change triggered indirectly. Would appreciate your guidance on how to proceed — happy to investigate further if you can point me in the right direction. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] fix: remove trigger event payload from INFO log in run_trigger() [airflow]
Feudjo commented on PR #67244: URL: https://github.com/apache/airflow/pull/67244#issuecomment-4644224904 > Can you add test and enable opportunity to print the event in debug mode (can be protected with ifs to avoid unnecessary serialization). Ideally also add some tests mocking the behaviour to avoid regression and make a comment in the code explaining why we are doing it @potiuk Thanks for the review. I've addressed the comments: - Printing log in debug mode - Added unit test. Could you please take another look when you have time? -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] fix: remove trigger event payload from INFO log in run_trigger() [airflow]
potiuk commented on PR #67244: URL: https://github.com/apache/airflow/pull/67244#issuecomment-4582718081 Can you add test and enable opportunity to print the event in debug mode (can be protected with ifs to avoid unnecessary serialization). Ideally also add some tests mocking the behaviour to avoid regression and make a comment in the code explaining why we are doing 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. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] fix: remove trigger event payload from INFO log in run_trigger() [airflow]
Feudjo commented on PR #67244: URL: https://github.com/apache/airflow/pull/67244#issuecomment-4582426089 @potiuk ready for review. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] fix: remove trigger event payload from INFO log in run_trigger() [airflow]
potiuk commented on PR #67244: URL: https://github.com/apache/airflow/pull/67244#issuecomment-4529640464 @Feudjo A few things need addressing before review — see our [Pull Request quality criteria](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#pull-request-quality-criteria). - :x: **CI fails**: `CI image checks / Static checks` (and possibly other checks — see the [Checks tab](https://github.com/apache/airflow/pull/67244/checks) for the full list). No rush. --- _Note: This comment was drafted by an AI-assisted triage tool and may contain mistakes. Once you have addressed the points above, an Apache Airflow maintainer — a real person — will take the next look at your PR. We use this [two-stage triage process](https://github.com/apache/airflow/blob/main/contributing-docs/25_maintainer_pr_triage.md#why-the-first-pass-is-automated) so that our maintainers' limited time is spent where it matters most: the conversation with you._ --- Drafted-by: Claude Code (Opus 4.7); reviewed by @potiuk before posting -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] fix: remove trigger event payload from INFO log in run_trigger() [airflow]
boring-cyborg[bot] commented on PR #67244: URL: https://github.com/apache/airflow/pull/67244#issuecomment-4499333123 Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our [Contributors' Guide](https://github.com/apache/airflow/blob/main/contributing-docs/README.rst) Here are some useful points: - Pay attention to the quality of your code (ruff, mypy and type annotations). Our [prek-hooks]( https://github.com/apache/airflow/blob/main/contributing-docs/08_static_code_checks.rst#prerequisites-for-prek-hooks) will help you with that. - In case of a new feature add useful documentation (in docstrings or in `docs/` directory). Adding a new operator? Check this short [guide](https://github.com/apache/airflow/blob/main/airflow-core/docs/howto/custom-operator.rst) Consider adding an example Dag that shows how users should use it. - Consider using [Breeze environment](https://github.com/apache/airflow/blob/main/dev/breeze/doc/README.rst) for testing locally, it's a heavy docker but it ships with a working Airflow and a lot of integrations. - Be patient and persistent. It might take some time to get a review or get the final approval from Committers. - Please follow [ASF Code of Conduct](https://www.apache.org/foundation/policies/conduct) for all communication including (but not limited to) comments on Pull Requests, Mailing list and Slack. - Be sure to read the [Airflow Coding style]( https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#coding-style-and-best-practices). - Always keep your Pull Requests rebased, otherwise your build might fail due to changes not related to your commits. Apache Airflow is a community-driven project and together we are making it better 🚀. In case of doubts contact the developers at: Mailing List: [email protected] Slack: https://s.apache.org/airflow-slack -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
