Skip to content

Commit

Permalink
Raise 404 from Variable PATCH API if variable is not found (#33885)
Browse files Browse the repository at this point in the history
* Raise variable not found if session returns empty

* Added detail to the exception for json reponse

* tests for patch api when variable doesn't exist

* Dropped fstring

* Unify varialbe not found message

---------

Co-authored-by: Tzu-ping Chung <[email protected]>
  • Loading branch information
abhishekbhakat and uranusjr authored Aug 30, 2023
1 parent d361761 commit 701c3b8
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 1 deletion.
4 changes: 3 additions & 1 deletion airflow/api_connexion/endpoints/variable_endpoint.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ def get_variable(*, variable_key: str, session: Session = NEW_SESSION) -> Respon
"""Get a variable by key."""
var = session.scalar(select(Variable).where(Variable.key == variable_key).limit(1))
if not var:
raise NotFound("Variable not found")
raise NotFound("Variable not found", detail="Variable does not exist")
return variable_schema.dump(var)


Expand Down Expand Up @@ -116,6 +116,8 @@ def patch_variable(
raise BadRequest("Invalid post body", detail="key from request body doesn't match uri parameter")
non_update_fields = ["key"]
variable = session.scalar(select(Variable).filter_by(key=variable_key).limit(1))
if not variable:
raise NotFound("Variable not found", detail="Variable does not exist")
if update_mask:
data = extract_update_mask_data(update_mask, non_update_fields, data)
for key, val in data.items():
Expand Down
15 changes: 15 additions & 0 deletions tests/api_connexion/endpoints/test_variable_endpoint.py
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,21 @@ def test_should_update_variable_with_mask(self, session):
_check_last_log(session, dag_id=None, event="variable.edit", execution_date=None)

def test_should_reject_invalid_update(self):
response = self.client.patch(
"/api/v1/variables/var1",
json={
"key": "var1",
"value": "foo",
},
environ_overrides={"REMOTE_USER": "test"},
)
assert response.status_code == 404
assert response.json == {
"title": "Variable not found",
"status": 404,
"type": EXCEPTIONS_LINK_MAP[404],
"detail": "Variable does not exist",
}
Variable.set("var1", "foo")
response = self.client.patch(
"/api/v1/variables/var1",
Expand Down

0 comments on commit 701c3b8

Please sign in to comment.