[PR] Adding 1 second sleep time between retries for publishing tasks [airflow]

["sathish9094 (via GitHub)" <gi...@apache.org>]

sathish9094 opened a new pull request, #35791:
URL: https://github.com/apache/airflow/pull/35791

   This retry loop is pretty fast and it doesn't wait for number of seconds value that we pass from airflow.cfg
   ```
   # The Maximum number of retries for publishing task messages to the broker when failing
   # due to ``AirflowTaskTimeout`` error before giving up and marking Task as failed.
   task_publish_max_retries = 3
   ```
   By making it to wait for a second before each retry will give some time for broker recovery before making the task as failed.
   


-- 
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-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Adding 1 second sleep time between retries for publishing tasks [airflow]

["potiuk (via GitHub)" <gi...@apache.org>]

potiuk commented on PR #35791:
URL: https://github.com/apache/airflow/pull/35791#issuecomment-1835835446

   I don't think hardcoding it to 1 second is a good idea. it sounds pretty magical. Also it has potentially undesireable side effects - there might be cases where the retry is actually good if it happens quickly.
   
   If we want to do add anything like that, I'd say we should have a configuration option telling whether to delay retries or not and how long. Also it is generally a bad idea to have a fixed hard-coded value - ideally there should be a way to get at least a bit of exponential back-off and make several retries with increasing delay period  - and we should be able to configure both initlal delay, number of retries and exponential back-off factor. 
   
   We can use tenacity for that - we already use it for similar purposes.
   
   I think if this is done this way - with configurable retries, exponential backoff (and unit tests as well) - then we could merge it I think.


-- 
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-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Adding 1 second sleep time between retries for publishing tasks [airflow]

["github-actions[bot] (via GitHub)" <gi...@apache.org>]

github-actions[bot] closed pull request #35791: Adding 1 second sleep time between retries for publishing tasks
URL: https://github.com/apache/airflow/pull/35791


-- 
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-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Adding 1 second sleep time between retries for publishing tasks [airflow]

["boring-cyborg[bot] (via GitHub)" <gi...@apache.org>]

boring-cyborg[bot] commented on PR #35791:
URL: https://github.com/apache/airflow/pull/35791#issuecomment-1822301201

   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 Contribution Guide (https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst)
   Here are some useful points:
   - Pay attention to the quality of your code (ruff, mypy and type annotations). Our [pre-commits]( https://github.com/apache/airflow/blob/main/STATIC_CODE_CHECKS.rst#prerequisites-for-pre-commit-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/docs/apache-airflow/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/BREEZE.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.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: dev@airflow.apache.org
   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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Adding 1 second sleep time between retries for publishing tasks [airflow]

["sathish9094 (via GitHub)" <gi...@apache.org>]

sathish9094 commented on PR #35791:
URL: https://github.com/apache/airflow/pull/35791#issuecomment-1825122613

   @hussein-awala Can you please help to review/approve 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.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org