Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add separate error handler for 405(Method not allowed) errors #26880

Merged
merged 1 commit into from Oct 18, 2022
Merged

Add separate error handler for 405(Method not allowed) errors #26880

merged 1 commit into from Oct 18, 2022

Conversation

dakshin-k
Copy link
Contributor

Previously, the webserver was using the same error handler for both 404 and 405 errors, and returning a 'Not Found' response for both cases. Hence the response status code was always 404, even when a 405(Method not allowed) was raised internally.

This PR creates a separate error handler for 405 errors, so it will return an appropriate response and status code.

closes: #26375

Results of manual testing

  • Returns a JSON response when hitting the REST APIs
dakshin@bluepenguin:~/projects/airflow$ curl -X PATCH -D - http://localhost:28080/api/v1/version
HTTP/1.1 405 METHOD NOT ALLOWED
Server: gunicorn
Date: Wed, 05 Oct 2022 08:35:30 GMT
Connection: close
Content-Type: application/problem+json
Content-Length: 142
X-Robots-Tag: noindex, nofollow
Set-Cookie: session=e2b057f0-12bb-4275-8d7c-ec10f86f291f.T49QxckJWF1FK5zE-nOHZMkMVg8; Expires=Fri, 04 Nov 2022 08:35:30 GMT; HttpOnly; Path=/; SameSite=Lax

{
  "detail": "The method is not allowed for the requested URL.",
  "status": 405,
  "title": "Method Not Allowed",
  "type": "about:blank"
}
  • Returns a HTML response for other URLs
dakshin@bluepenguin:~/projects/airflow$ curl -X PATCH -D - http://localhost:28080/home
HTTP/1.1 405 METHOD NOT ALLOWED
Server: Werkzeug/2.2.2 Python/3.7.14
Date: Wed, 05 Oct 2022 08:42:30 GMT
Content-Type: text/html; charset=utf-8
Content-Length: 475
X-Robots-Tag: noindex, nofollow
Set-Cookie: session=1b653de3-8dfa-477f-a4b5-72bbef8bfaa9.vuiHP-jyaAoquKD_OAd5DEdQVmQ; Expires=Fri, 04 Nov 2022 08:42:30 GMT; HttpOnly; Path=/; SameSite=Lax
Connection: close



<!DOCTYPE html>
<html lang="en">
  <head>
    <title>Airflow 405</title>
    <link rel="icon" type="image/png" href="/static/pin_32.png">
  </head>
  <body>
    <div style="font-family: verdana; text-align: center; margin-top: 200px;">
      <img src="/static/pin_100.png" width="50px" alt="pin-logo" />
      <h1>Airflow 405</h1>
      <p>Received an invalid request.</p>
      <a href="/">Return to the main page</a>
      <p>bdb986291012</p>
    </div>
  </body>
</html>

@boring-cyborg boring-cyborg bot added area:API Airflow's REST/HTTP API area:UI Related to UI/UX. For Frontend Developers. area:webserver Webserver related Issues labels Oct 5, 2022
@boring-cyborg
Copy link

boring-cyborg bot commented Oct 5, 2022

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 (flake8, mypy and type annotations). Our pre-commits 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 Consider adding an example DAG that shows how users should use it.
  • Consider using Breeze environment 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 for all communication including (but not limited to) comments on Pull Requests, Mailing list and Slack.
  • Be sure to read the Airflow Coding style.
    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

@dakshin-k dakshin-k marked this pull request as ready for review October 5, 2022 11:01
uranusjr
uranusjr reviewed Oct 6, 2022
View reviewed changes
airflow/www/views.py Outdated Show resolved Hide resolved
@dakshin-k dakshin-k force-pushed the 405-error-response branch 2 times, most recently from dd41868 to f2d1fc8 Compare October 8, 2022 08:37
@dakshin-k dakshin-k requested review from potiuk and uranusjr and removed request for ryanahamilton, ashb, bbovenzi, potiuk and uranusjr October 9, 2022 05:39
@potiuk
Copy link
Member

potiuk commented Oct 10, 2022

Static checks are failing - I suggest to install pre-commit locally (it will even fix things for you and running breeze static-checks --last-commit should run pre-commit on latest commit.

BTW. I will not be available for next two weeks (vacations) so I hope @uranusjr or someone else will merge it before :)

@dakshin-k
Copy link
Contributor Author

Fixed the static check issues, need approval to run the workflow again.

@uranusjr, do take a look if you can 😄

@eladkal eladkal added this to the Airflow 2.4.2 milestone Oct 17, 2022
@ephraimbuddy ephraimbuddy merged commit 8efb678 into apache:main Oct 18, 2022
@boring-cyborg
Copy link

boring-cyborg bot commented Oct 18, 2022

Awesome work, congrats on your first merged pull request!

@ephraimbuddy ephraimbuddy added the type:bug-fix Changelog: Bug Fixes label Oct 18, 2022
ephraimbuddy pushed a commit that referenced this pull request Oct 18, 2022
@dakshin-k @ephraimbuddy
Add separate error handler for 405(Method not allowed) errors (#26880)
5bf2850 
Co-authored-by: Dakshin K <dakshin.k1@gmail.com>
(cherry picked from commit 8efb678)
@dakshin-k dakshin-k deleted the 405-error-response branch October 18, 2022 21:48
ephraimbuddy pushed a commit that referenced this pull request Oct 18, 2022
@dakshin-k @ephraimbuddy
Add separate error handler for 405(Method not allowed) errors (#26880)
42176dc 
Co-authored-by: Dakshin K <dakshin.k1@gmail.com>
(cherry picked from commit 8efb678)
@ephraimbuddy ephraimbuddy mentioned this pull request Oct 20, 2022
Closed
37 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@potiuk potiuk potiuk approved these changes

@uranusjr uranusjr uranusjr approved these changes

@ephraimbuddy ephraimbuddy ephraimbuddy approved these changes

Assignees
No one assigned
Labels
area:API Airflow's REST/HTTP API area:UI Related to UI/UX. For Frontend Developers. area:webserver Webserver related Issues type:bug-fix Changelog: Bug Fixes
Projects
None yet
Milestone
Airflow 2.4.2
Development

Successfully merging this pull request may close these issues.

Airflow Webserver returns incorrect HTTP Error Response for custom REST API endpoints
5 participants
@dakshin-k @potiuk @uranusjr @ephraimbuddy @eladkal