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

Set strict to True when parsing dates in webserver views #33512

Merged
merged 5 commits into from
Aug 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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