You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2020/10/17 20:29:36 UTC
[GitHub] [airflow] jmcarp opened a new pull request #11620: Check response status in slack webhook hook.
jmcarp opened a new pull request #11620:
URL: https://github.com/apache/airflow/pull/11620
The slack webhook hook doesn't check the status code of the http response. I think this isn't the right behavior. For example, if the user enters the wrong webhook token, they'll get a 403, but the hook won't crash. This patch passes `check_response` to the superclass `run` method so that the hook fails on errors.
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#pull-request-guidelines)** for more information.
In case of fundamental code change, Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvements+Proposals)) is needed.
In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
In case of backwards incompatible changes please leave a note in [UPDATING.md](https://github.com/apache/airflow/blob/master/UPDATING.md).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] jmcarp commented on pull request #11620: Check response status in slack webhook hook.
Posted by GitBox <gi...@apache.org>.
jmcarp commented on pull request #11620:
URL: https://github.com/apache/airflow/pull/11620#issuecomment-711485023
I think of this as a bug fix and not a breaking change. I think users would be surprised to find that an operator that sends an http request doesn't fail when the response is a 403. But if there are users relying on the current behavior for some reason, we can add a `check_response` flag that defaults to `False`. I'm just not sure when I wouldn't want to check for an error here.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] turbaszek merged pull request #11620: Check response status in Slack webhook hook
Posted by GitBox <gi...@apache.org>.
turbaszek merged pull request #11620:
URL: https://github.com/apache/airflow/pull/11620
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org