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.

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

DryRun is not optional for patch task instance #34563

Closed
Calder-Ty opened this issue Sep 22, 2023 · 3 comments · Fixed by #34568
Closed

DryRun is not optional for patch task instance #34563

Calder-Ty opened this issue Sep 22, 2023 · 3 comments · Fixed by #34568
Labels
area:API Airflow's REST/HTTP API kind:bug This is a clearly a bug kind:documentation

Comments

@Calder-Ty
Copy link
Contributor

Summary

According to the REST api docs.
you can patch a task instance state. When you hit the api without sending a
"dry_run" variable, you get a KeyError (This is from a server running version 2.5.3):

Traceback (most recent call last):
  File "/home/airflow/.local/lib/python3.10/site-packages/flask/app.py", line 2528, in wsgi_app
    response = self.full_dispatch_request()
  File "/home/airflow/.local/lib/python3.10/site-packages/flask/app.py", line 1825, in full_dispatch_request
    rv = self.handle_user_exception(e)
  File "/home/airflow/.local/lib/python3.10/site-packages/flask/app.py", line 1823, in full_dispatch_request
    rv = self.dispatch_request()
  File "/home/airflow/.local/lib/python3.10/site-packages/flask/app.py", line 1799, in dispatch_request
    return self.ensure_sync(self.view_functions[rule.endpoint])(**view_args)
  File "/home/airflow/.local/lib/python3.10/site-packages/connexion/decorators/decorator.py", line 68, in wrapper
    response = function(request)
  File "/home/airflow/.local/lib/python3.10/site-packages/connexion/decorators/uri_parsing.py", line 149, in wrapper
    response = function(request)
  File "/home/airflow/.local/lib/python3.10/site-packages/connexion/decorators/validation.py", line 196, in wrapper
    response = function(request)
  File "/home/airflow/.local/lib/python3.10/site-packages/connexion/decorators/validation.py", line 399, in wrapper
    return function(request)
  File "/home/airflow/.local/lib/python3.10/site-packages/connexion/decorators/response.py", line 112, in wrapper
    response = function(request)
  File "/home/airflow/.local/lib/python3.10/site-packages/connexion/decorators/parameter.py", line 120, in wrapper
    return function(**kwargs)
  File "/home/airflow/.local/lib/python3.10/site-packages/airflow/api_connexion/security.py", line 51, in decorated
    return func(*args, **kwargs)
  File "/home/airflow/.local/lib/python3.10/site-packages/airflow/utils/session.py", line 75, in wrapper
    return func(*args, session=session, **kwargs)
  File "/home/airflow/.local/lib/python3.10/site-packages/airflow/api_connexion/endpoints/task_instance_endpoint.py", line 594, in patch_task_instance
    if not data["dry_run"]:
KeyError: 'dry_run'

The API docs state that dry_run is not required and that it is defaulted to false.

This can be reproduced in main with the tests by commenting out line 1699 in
test_task_instance_endpoint.py

@boring-cyborg
Copy link

boring-cyborg bot commented Sep 22, 2023

Thanks for opening your first issue here! Be sure to follow the issue template! If you are willing to raise PR to address this issue please do so, no need to wait for approval.

@f0restron07
Copy link

Hi,

Thank you for sharing this. It appears there is a mismatch between the API documentation and the actual behavior of the endpoint, which is resulting in a KeyError for the dry_run variable.

Suggested Steps:
Bug Report:
If not already done, consider reporting this as a bug in the official Airflow GitHub repository or check if there is already an existing issue reported for this behavior.

Temporary Workaround:
As a temporary workaround, you can include the dry_run variable with a value of false in your API requests to avoid the KeyError, until the issue is fixed in the codebase.

Code Modification:
You can also modify the code to handle the absence of the dry_run variable properly, making it optional as per the documentation, and set it to false as the default value if it's not provided in the request.

Example Code Modification:

data.get("dry_run", False)
This would access the dry_run value if it exists in the data dictionary and would default to False if it doesn’t exist, avoiding the KeyError.

Next Steps:
Await for feedback on the submitted bug report.
Follow the status of the issue and apply the official fix once it’s available in the future release.
In the meantime, it would be helpful to always provide the dry_run variable with your requests as mentioned above.

I hope this helps and makes your interaction with Airflow’s REST API smoother.

Best Regards,
f0restron

@hussein-awala
Copy link
Member

@f0restron07 Please avoid the answers generated with LLM, if you have a valuable comments, don't hesitate to add them, but copying an LLM answer could have a negative impact on communication with users

If not already done, consider reporting this as a bug in the official Airflow GitHub repository or check if there is already an existing issue reported for this behavior.

This issue is what we call bug report :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:API Airflow's REST/HTTP API kind:bug This is a clearly a bug kind:documentation
Projects
None yet
3 participants