Skip to content

Commit

Permalink
Set strict to True when parsing dates in webserver views (#33512)
Browse files Browse the repository at this point in the history
Co-authored-by: Tzu-ping Chung <uranusjr@gmail.com>

---------

Co-authored-by: Tzu-ping Chung <uranusjr@gmail.com>
  • Loading branch information
hussein-awala and uranusjr committed Aug 20, 2023
1 parent 7700fb1 commit 4390524
Show file tree
Hide file tree
Showing 5 changed files with 25 additions and 19 deletions.
5 changes: 3 additions & 2 deletions airflow/utils/timezone.py
Original file line number Diff line number Diff line change
Expand Up @@ -194,16 +194,17 @@ def datetime(*args, **kwargs):
return dt.datetime(*args, **kwargs)


def parse(string: str, timezone=None) -> DateTime:
def parse(string: str, timezone=None, *, strict=False) -> DateTime:
"""
Parse a time string and return an aware datetime.
:param string: time string
:param timezone: the timezone
:param strict: if False, it will fall back on the dateutil parser if unable to parse with pendulum
"""
from airflow.settings import TIMEZONE

return pendulum.parse(string, tz=timezone or TIMEZONE, strict=False) # type: ignore
return pendulum.parse(string, tz=timezone or TIMEZONE, strict=strict) # type: ignore


@overload
Expand Down
11 changes: 6 additions & 5 deletions airflow/www/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -265,17 +265,18 @@ def get_date_time_num_runs_dag_runs_form_data(www_request, session, dag):
}


def _safe_parse_datetime(v, allow_empty=False) -> datetime.datetime | None:
def _safe_parse_datetime(v, *, allow_empty=False, strict=True) -> datetime.datetime | None:
"""
Parse datetime and return error message for invalid dates.
:param v: the string value to be parsed
:param allow_empty: Set True to return none if empty str or None
:param strict: if False, it will fall back on the dateutil parser if unable to parse with pendulum
"""
if allow_empty is True and not v:
return None
try:
return timezone.parse(v)
return timezone.parse(v, strict=strict)
except (TypeError, ParserError):
abort(400, f"Invalid datetime: {v!r}")

Expand Down Expand Up @@ -1638,7 +1639,7 @@ def get_logs_with_metadata(self, session: Session = NEW_SESSION):

# Convert string datetime into actual datetime
try:
execution_date = timezone.parse(execution_date_str)
execution_date = timezone.parse(execution_date_str, strict=True)
except ValueError:
error_message = (
f"Given execution date, {execution_date}, could not be identified as a date. "
Expand Down Expand Up @@ -2111,7 +2112,7 @@ def trigger(self, dag_id: str, session: Session = NEW_SESSION):
)

try:
execution_date = timezone.parse(request_execution_date)
execution_date = timezone.parse(request_execution_date, strict=True)
except ParserError:
flash("Invalid execution date", "error")
form = DateTimeForm(data={"execution_date": timezone.utcnow().isoformat()})
Expand Down Expand Up @@ -3707,7 +3708,7 @@ def grid_data(self):
num_runs = conf.getint("webserver", "default_dag_run_display_number")

try:
base_date = timezone.parse(request.args["base_date"])
base_date = timezone.parse(request.args["base_date"], strict=True)
except (KeyError, ValueError):
base_date = dag.get_latest_execution_date() or timezone.utcnow()

Expand Down
22 changes: 12 additions & 10 deletions tests/www/views/test_views_extra_links.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
from __future__ import annotations

import json
import urllib.parse
from unittest import mock

import pytest
Expand All @@ -30,7 +31,8 @@
from tests.test_utils.db import clear_db_runs
from tests.test_utils.mock_operators import AirflowLink, Dummy2TestOperator, Dummy3TestOperator

DEFAULT_DATE = timezone.datetime(2017, 1, 1)
DEFAULT_DATE = timezone.datetime(2017, 1, 1, tzinfo=timezone.utc)
STR_DEFAULT_DATE = urllib.parse.quote(DEFAULT_DATE.strftime("%Y-%m-%dT%H:%M:%S.%f%z"))

ENDPOINT = "extra_links"

Expand Down Expand Up @@ -129,7 +131,7 @@ def reset_task_instances():
def test_extra_links_works(dag_run, task_1, viewer_client, session):
response = viewer_client.get(
f"{ENDPOINT}?dag_id={task_1.dag_id}&task_id={task_1.task_id}"
f"&execution_date={DEFAULT_DATE}&link_name=foo-bar",
f"&execution_date={STR_DEFAULT_DATE}&link_name=foo-bar",
follow_redirects=True,
)

Expand All @@ -143,7 +145,7 @@ def test_extra_links_works(dag_run, task_1, viewer_client, session):
def test_global_extra_links_works(dag_run, task_1, viewer_client, session):
response = viewer_client.get(
f"{ENDPOINT}?dag_id={dag_run.dag_id}&task_id={task_1.task_id}"
f"&execution_date={DEFAULT_DATE}&link_name=github",
f"&execution_date={STR_DEFAULT_DATE}&link_name=github",
follow_redirects=True,
)

Expand All @@ -157,7 +159,7 @@ def test_global_extra_links_works(dag_run, task_1, viewer_client, session):
def test_operator_extra_link_override_global_extra_link(dag_run, task_1, viewer_client):
response = viewer_client.get(
f"{ENDPOINT}?dag_id={task_1.dag_id}&task_id={task_1.task_id}"
f"&execution_date={DEFAULT_DATE}&link_name=airflow",
f"&execution_date={STR_DEFAULT_DATE}&link_name=airflow",
follow_redirects=True,
)

Expand All @@ -171,7 +173,7 @@ def test_operator_extra_link_override_global_extra_link(dag_run, task_1, viewer_
def test_extra_links_error_raised(dag_run, task_1, viewer_client):
response = viewer_client.get(
f"{ENDPOINT}?dag_id={task_1.dag_id}&task_id={task_1.task_id}"
f"&execution_date={DEFAULT_DATE}&link_name=raise_error",
f"&execution_date={STR_DEFAULT_DATE}&link_name=raise_error",
follow_redirects=True,
)

Expand All @@ -185,7 +187,7 @@ def test_extra_links_error_raised(dag_run, task_1, viewer_client):
def test_extra_links_no_response(dag_run, task_1, viewer_client):
response = viewer_client.get(
f"{ENDPOINT}?dag_id={task_1.dag_id}&task_id={task_1.task_id}"
f"&execution_date={DEFAULT_DATE}&link_name=no_response",
f"&execution_date={STR_DEFAULT_DATE}&link_name=no_response",
follow_redirects=True,
)

Expand All @@ -206,7 +208,7 @@ def test_operator_extra_link_override_plugin(dag_run, task_2, viewer_client):
"""
response = viewer_client.get(
f"{ENDPOINT}?dag_id={task_2.dag_id}&task_id={task_2.task_id}"
f"&execution_date={DEFAULT_DATE}&link_name=airflow",
f"&execution_date={STR_DEFAULT_DATE}&link_name=airflow",
follow_redirects=True,
)

Expand All @@ -228,7 +230,7 @@ def test_operator_extra_link_multiple_operators(dag_run, task_2, task_3, viewer_
"""
response = viewer_client.get(
f"{ENDPOINT}?dag_id={task_2.dag_id}&task_id={task_2.task_id}"
f"&execution_date={DEFAULT_DATE}&link_name=airflow",
f"&execution_date={STR_DEFAULT_DATE}&link_name=airflow",
follow_redirects=True,
)

Expand All @@ -240,7 +242,7 @@ def test_operator_extra_link_multiple_operators(dag_run, task_2, task_3, viewer_

response = viewer_client.get(
f"{ENDPOINT}?dag_id={task_3.dag_id}&task_id={task_3.task_id}"
f"&execution_date={DEFAULT_DATE}&link_name=airflow",
f"&execution_date={STR_DEFAULT_DATE}&link_name=airflow",
follow_redirects=True,
)

Expand All @@ -253,7 +255,7 @@ def test_operator_extra_link_multiple_operators(dag_run, task_2, task_3, viewer_
# Also check that the other Operator Link defined for this operator exists
response = viewer_client.get(
f"{ENDPOINT}?dag_id={task_3.dag_id}&task_id={task_3.task_id}"
f"&execution_date={DEFAULT_DATE}&link_name=google",
f"&execution_date={STR_DEFAULT_DATE}&link_name=google",
follow_redirects=True,
)

Expand Down
3 changes: 2 additions & 1 deletion tests/www/views/test_views_log.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,8 @@
DAG_ID_REMOVED = "removed_dag_for_testing_log_view"
TASK_ID = "task_for_testing_log_view"
DEFAULT_DATE = timezone.datetime(2017, 9, 1)
ENDPOINT = f"log?dag_id={DAG_ID}&task_id={TASK_ID}&execution_date={DEFAULT_DATE}"
STR_DEFAULT_DATE = urllib.parse.quote(DEFAULT_DATE.strftime("%Y-%m-%dT%H:%M:%S.%f%z"))
ENDPOINT = f"log?dag_id={DAG_ID}&task_id={TASK_ID}&execution_date={STR_DEFAULT_DATE}"


@pytest.fixture(scope="module", autouse=True)
Expand Down
3 changes: 2 additions & 1 deletion tests/www/views/test_views_tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
from tests.test_utils.www import check_content_in_response, check_content_not_in_response, client_with_login

DEFAULT_DATE = timezone.utcnow().replace(hour=0, minute=0, second=0, microsecond=0)
STR_DEFAULT_DATE = urllib.parse.quote(DEFAULT_DATE.strftime("%Y-%m-%dT%H:%M:%S.%f%z"))

DEFAULT_VAL = urllib.parse.quote_plus(str(DEFAULT_DATE))

Expand Down Expand Up @@ -1041,7 +1042,7 @@ def test_graph_view_doesnt_fail_on_recursion_error(app, dag_maker, admin_client)
def test_task_instances(admin_client):
"""Test task_instances view."""
resp = admin_client.get(
f"/object/task_instances?dag_id=example_bash_operator&execution_date={DEFAULT_DATE}",
f"/object/task_instances?dag_id=example_bash_operator&execution_date={STR_DEFAULT_DATE}",
follow_redirects=True,
)
assert resp.status_code == 200
Expand Down

0 comments on commit 4390524

Please sign in to comment.