Skip to content
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
1 change: 1 addition & 0 deletions backend/kernelCI_app/constants/localization.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ class ClientStrings:
ISSUE_NOT_FOUND = "Issue not found"
NO_ISSUE_FOUND = "No issues found"
INVALID_JSON_BODY = "Invalid body, request body must be a valid json string"
INVALID_FILTERS = "Invalid filter key or value"
ISSUE_EMPTY_LIST = "Invalid body, the issue list must not be empty"
ISSUE_NO_EXTRA_DETAILS = (
"No extra details found. Issue id has no incident or doesn't exist."
Expand Down
18 changes: 18 additions & 0 deletions backend/kernelCI_app/helpers/filters.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
from kernelCI_app.constants.general import UNCATEGORIZED_STRING
from kernelCI_app.helpers.commonDetails import PossibleTabs
from kernelCI_app.helpers.logger import log_message
from kernelCI_app.models import StatusChoices
from kernelCI_app.typeModels.databases import (
StatusValues,
failure_status_list,
Expand All @@ -22,6 +23,10 @@
NULL_STRINGS = set(["null", UNKNOWN_STRING, "NULL"])


def is_valid_status(status: str) -> bool:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 bad{*StatusChoices, "NULL"} reconstructs the set on every call, and if status is None, status.upper() will raise AttributeError.

Suggestion:

VALID_STATUSES = {choice.value for choice in StatusChoices} | {"NULL"}

def is_valid_status(status: str) -> bool:
    return status is not None and status.upper() in VALID_STATUSES

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This shouldn't be a problem, as it is not in any hot path of the application, also the set is quite small.
The status is a str, not Optional[str], it should not be None.

return status.upper() in {*StatusChoices, "NULL"}


def is_status_failure(
test_status: StatusValues, fail_list: list[StatusValues] = failure_status_list
) -> bool:
Expand Down Expand Up @@ -129,6 +134,10 @@ def is_issue_filtered_out(
return not in_filter


def is_filtered_out(value: str, filter_values: set[str]):
return filter_values and value not in filter_values


def should_filter_test_issue(
*,
issue_filters: set,
Expand Down Expand Up @@ -361,6 +370,7 @@ def __init__(self, data: Dict, process_body=False) -> None:
"test.status": self._handle_test_status,
"test.duration": self._handle_test_duration,
"build.status": self._handle_build_status,
Comment thread
MarceloRobert marked this conversation as resolved.
"duration": self._handle_build_duration, # TODO: same as build.duration (should be standardized)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 nit — If this "duration" alias is known tech debt, consider creating a ticket instead of a TODO — it may confuse future contributors about which filter key to use.

"build.duration": self._handle_build_duration,
"origin": self._handle_origins,
"config_name": self._handle_config_name,
Expand Down Expand Up @@ -391,6 +401,14 @@ def __init__(self, data: Dict, process_body=False) -> None:

self._process_filters()

def __repr__(self) -> str:
parts = ""
for parsed_filter in self.filters:
parts += "\n\t{},".format(
", ".join([f"{key}={val}" for key, val in parsed_filter.items()])
)
return f"FilterParams({parts})"

def _handle_boot_status(self, current_filter: ParsedFilter) -> None:
self.filterBootStatus.add(current_filter["value"])

Expand Down
4 changes: 2 additions & 2 deletions backend/kernelCI_app/helpers/hardwareDetails.py
Original file line number Diff line number Diff line change
Expand Up @@ -509,8 +509,8 @@ def process_issue(
incident_test_id=record["incidents__test_id"],
build_status=record["build__status"],
test_status=record["status"],
issue_comment=record["incidents__issue__comment"],
issue_report_url=record["incidents__issue__report_url"],
issue_comment=record.get("incidents__issue__comment"),
issue_report_url=record.get("incidents__issue__report_url"),
is_failed_task=is_failed_task,
issue_from=issue_from,
task=task_issues_dict,
Expand Down
11 changes: 10 additions & 1 deletion backend/kernelCI_app/helpers/issueExtras.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from collections import defaultdict
from typing import List, Tuple
from typing import List, Tuple, Optional

from kernelCI_app.constants.general import UNCATEGORIZED_STRING
from kernelCI_app.helpers.logger import log_message
from kernelCI_app.queries.issues import get_issue_first_seen_data, get_issue_trees_data
from kernelCI_app.typeModels.issues import (
Expand All @@ -20,6 +21,14 @@ class TagUrls:
)


def parse_issue(issue_str: Optional[str]) -> tuple[str, Optional[int]]:
if issue_str is None:
return (UNCATEGORIZED_STRING, None)
issue_id, _, issue_version = issue_str.partition(",")
issue_version = int(issue_version) if issue_version.upper() != "NULL" else None
return (issue_id, issue_version)


def process_issues_extra_details(
*,
issue_key_list: List[Tuple[str, int]],
Expand Down
Loading
Loading