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
23 changes: 0 additions & 23 deletions .github/workflows/lint.yml

This file was deleted.

10 changes: 10 additions & 0 deletions .github/workflows/linters.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
name: Linters

on:
push:
branches: ["master"]
Comment on lines +4 to +5
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Switch the push trigger to main.

Line 5 still points at master, but this PR is targeting main. After merge, pushes to main won't execute the lint workflow.

Suggested fix
 on:
   push:
-    branches: ["master"]
+    branches: ["main"]
   pull_request:
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
push:
branches: ["master"]
push:
branches: ["main"]
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/linters.yaml around lines 4 - 5, Update the GitHub Actions
push trigger branches under the "push" workflow entry by replacing the current
branch name "master" with "main" so pushes to main will run the lint workflow;
locate the branches array (branches: ["master"]) and change it to branches:
["main"].

pull_request:

jobs:
lint:
uses: RedHatInsights/processing-tools/.github/workflows/linters.yaml@v0.1.0
88 changes: 54 additions & 34 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
@@ -1,36 +1,56 @@
exclude: "doc|research|demos"
repos:
- repo: https://github.com/psf/black
rev: 26.3.1
hooks:
- id: black
args: [--safe, --quiet, --line-length, "100"]
language_version: python3.11
require_serial: true
- repo: https://github.com/pre-commit/pre-commit-hooks
rev: v6.0.0
hooks:
- id: trailing-whitespace
language_version: python3.11
- id: end-of-file-fixer
language_version: python3.11
- id: check-yaml
language_version: python3.11
- id: debug-statements
language_version: python3.11
- repo: https://github.com/asottile/pyupgrade
rev: v3.21.2
hooks:
- id: pyupgrade
language_version: python3.11
- repo: https://github.com/PyCQA/flake8
rev: 7.3.0
hooks:
- id: flake8
language_version: python3.11
args: [--extend-ignore=E501]
- repo: https://gitlab.com/pycqa/pydocstyle
rev: 6.1.1
hooks:
- id: pydocstyle
language_version: python3.11
- repo: https://github.com/pre-commit/pre-commit-hooks
rev: v6.0.0
hooks:
- id: end-of-file-fixer
- id: trailing-whitespace
- id: check-json
- id: check-toml
- id: check-yaml
- id: debug-statements
- id: mixed-line-ending
- id: check-ast
- id: check-merge-conflict
- id: check-added-large-files

- repo: https://github.com/shellcheck-py/shellcheck-py
rev: v0.11.0.1
hooks:
- id: shellcheck
# Exclude checks that are commonly problematic in organization scripts
# SC1090: Can't follow non-constant source (dynamic sourcing)
# SC2086: Double quote to prevent globbing (intentional word splitting)
# SC2034: Variable appears unused (used by sourced scripts)
# SC1091: Not following included files (dynamic includes)
args: ['--exclude=SC1090,SC2086,SC2034,SC1091']

- repo: https://github.com/golangci/golangci-lint
rev: v2.11.3
hooks:
- id: golangci-lint-config-verify
- id: golangci-lint-full

- repo: https://github.com/RedHatInsights/processing-tools
rev: v0.1.0
hooks:
- id: abcgo
args: ['--threshold=64']

- id: go-version-consistency

- repo: https://github.com/astral-sh/ruff-pre-commit
rev: v0.15.5
hooks:
# Adding a set of rules to ruff check to match pyupgrade rules
- id: ruff-check
args:
- '--fix'
- '--line-length=100'
- '--select=UP,F632,E,W,F,I,UP,N,B,C4,SIM,D'
- id: ruff-format

Comment on lines +46 to +52
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect whether Ruff settings are already centralized elsewhere.
fd -HI 'pyproject\.toml|ruff\.toml|\.ruff\.toml' | while read -r file; do
  echo "== $file =="
  rg -n '(\[tool\.ruff|\[lint|\[format|line-length|select)' "$file" || true
done

Repository: RedHatInsights/ccx-upgrades-data-eng

Length of output: 189


🏁 Script executed:

cat -n pyproject.toml

Repository: RedHatInsights/ccx-upgrades-data-eng

Length of output: 445


🏁 Script executed:

cat -n ruff.toml

Repository: RedHatInsights/ccx-upgrades-data-eng

Length of output: 1381


Pre-commit Ruff configuration is out of sync with centralized ruff.toml.

The .pre-commit-config.yaml hook defines --select=UP,F632,E,W,F,I,UP,N,B,C4,SIM,D with --line-length=100, but ruff.toml defines a different set of lint rules (F,B,S,BLE,T10,EXE,RUF,PLE,A,E7,E9,W6,E711) with no line-length setting. Additionally, ruff-format has no args configured, and pyproject.toml defines max-line-length under [pycodestyle] instead of [tool.ruff].

Update .pre-commit-config.yaml to remove the hardcoded args and rely on the centralized ruff.toml configuration for both ruff-check and ruff-format. Alternatively, if the pre-commit rules are intentional, update ruff.toml to match and add the line-length setting.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.pre-commit-config.yaml around lines 46 - 53, The pre-commit hook entries
for the ruff hooks ('ruff-check' and 'ruff-format') are overriding the
centralized configuration; remove the hardcoded args block (the '--select=...'
and '--line-length=100' entries) from the 'ruff-check' hook and ensure
'ruff-format' has no custom args so both hooks defer to the centralized
ruff.toml/pyproject settings (or, if the intent is to keep those specific rules,
update ruff.toml/pyproject.toml to match the same rule set and max-line-length
instead of editing the pre-commit hook); target the 'ruff-check' and
'ruff-format' hook entries in .pre-commit-config.yaml when making the change.

# - repo: https://github.com/AleksaC/hadolint-py
Copy link
Contributor

Choose a reason for hiding this comment

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

any reason to keep this commented out?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I keep commented all of those that are in the "template" in the processing-tools repo, but are not easily fixable. Hadolint would require another task to be fixed, because we have warning in all our repositories

# rev: v2.14.0
# hooks:
# - id: hadolint
10 changes: 5 additions & 5 deletions ccx_upgrades_data_eng/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,11 @@
logger = logging.getLogger(__name__)


class SessionManagerException(Exception):
class SessionManagerError(Exception):
"""An exception related to the initialization of the session manager."""


class TokenException(Exception):
class TokenError(Exception):
"""An exception related to the refreshment of the SSO token."""


Expand All @@ -40,7 +40,7 @@ def __init__(
try:
oidc_config = requests.get(oauth_config_uri, verify=self.verify).json()
except Exception as ex:
raise SessionManagerException(
raise SessionManagerError(
f"Error getting the oauth config from the SSO server:\n{ex}"
) from ex

Expand All @@ -67,15 +67,15 @@ def refresh_token(self) -> str:
verify=self.verify,
)
except Exception as ex:
raise TokenException(f"Error refreshing the token:\n{ex}") from ex
raise TokenError(f"Error refreshing the token:\n{ex}") from ex

def get_session(self) -> OAuth2Session:
"""Return the OauthSession2 after refreshing the auth token."""
self.refresh_token()
return self.session


@lru_cache()
@lru_cache
def get_session_manager() -> Oauth2Manager:
"""Oauth2Manager cache."""
settings = get_settings()
Expand Down
3 changes: 2 additions & 1 deletion ccx_upgrades_data_eng/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import logging
from functools import lru_cache

from pydantic import ValidationError
from pydantic_settings import BaseSettings

Expand Down Expand Up @@ -44,7 +45,7 @@ class Settings(BaseSettings):
cache_size: int = DEFAULT_CACHE_SIZE


@lru_cache()
@lru_cache
def get_settings() -> Settings:
"""Create the Settings object for the cache."""
try:
Expand Down
8 changes: 6 additions & 2 deletions ccx_upgrades_data_eng/examples.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,15 @@
EXAMPLE_CONSOLE_URL = "https://console-openshift-console.some_url.com"

URL_ALERT = (
EXAMPLE_CONSOLE_URL + "/monitoring/alerts?orderBy=asc&sortBy=Severity&alert-name=" + ALERT_NAME
EXAMPLE_CONSOLE_URL
+ "/monitoring/alerts?orderBy=asc&sortBy=Severity&alert-name="
+ ALERT_NAME
)

URL_OPERATOR_CONDITIONS = (
EXAMPLE_CONSOLE_URL + "/k8s/cluster/config.openshift.io~v1~ClusterOperator/" + FOC_NAME
EXAMPLE_CONSOLE_URL
+ "/k8s/cluster/config.openshift.io~v1~ClusterOperator/"
+ FOC_NAME
)

EXAMPLE_PREDICTORS = {
Expand Down
11 changes: 7 additions & 4 deletions ccx_upgrades_data_eng/inference.py
Original file line number Diff line number Diff line change
@@ -1,16 +1,17 @@
"""Utils to interact with Inference service."""

import logging
from datetime import datetime, timezone

import requests
from fastapi import HTTPException
from cachetools import cached
from datetime import datetime, timezone
from fastapi import HTTPException

from ccx_upgrades_data_eng.config import get_settings
from ccx_upgrades_data_eng.models import (
InferenceResponse,
UpgradeApiResponse,
UpgradeRisksPredictors,
InferenceResponse,
UpgradeRisksPredictorsWithURLs,
)
from ccx_upgrades_data_eng.urls import fill_urls
Expand Down Expand Up @@ -41,7 +42,9 @@ def get_inference_for_predictors(

response = UpgradeApiResponse(
upgrade_recommended=calculate_upgrade_recommended(risks),
upgrade_risks_predictors=UpgradeRisksPredictorsWithURLs.model_validate(risks.model_dump()),
upgrade_risks_predictors=UpgradeRisksPredictorsWithURLs.model_validate(
risks.model_dump()
),
last_checked_at=datetime.now(tz=timezone.utc),
)
logger.debug("Inference response is: %s", response)
Expand Down
12 changes: 6 additions & 6 deletions ccx_upgrades_data_eng/logging_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,13 @@
# See the License for the specific language governing permissions and
# limitations under the License.

"""
Utility functions to redirect logs to cloudwatch.
"""Utility functions to redirect logs to cloudwatch."""

Copied from https://github.com/RedHatInsights/insights-ccx-messaging/blob/main/ccx_messaging/utils/logging.py # noqa: E501
"""
# Copied from https://github.com/RedHatInsights/insights-ccx-messaging/
# blob/main/ccx_messaging/utils/logging.py

import os
import logging
import os

from boto3.session import Session
from watchtower import CloudWatchLogHandler
Expand All @@ -35,6 +34,7 @@ def __new__(self):
logging.NullHandler: if the hanlder couldn't be configured.
or
watchtower.CloudWatchLogHandler: if it could be configured.

"""
enabled = os.getenv("LOGGING_TO_CW_ENABLED", "False").lower()
if enabled not in ("true", "1", "t", "yes"):
Expand All @@ -52,7 +52,7 @@ def __new__(self):
missing_envs = list(
filter(
lambda key: os.environ.get(key, "").strip() == "",
[key for key in aws_config_vars],
aws_config_vars,
)
)

Expand Down
45 changes: 28 additions & 17 deletions ccx_upgrades_data_eng/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,22 +2,24 @@

import logging
import os
from contextlib import asynccontextmanager
from uuid import UUID

from fastapi import Depends, FastAPI, Request, status
from fastapi.responses import JSONResponse
from contextlib import asynccontextmanager
from prometheus_fastapi_instrumentator import Instrumentator

import ccx_upgrades_data_eng.metrics as metrics
from ccx_upgrades_data_eng.auth import (
SessionManagerError,
TokenError,
get_session_manager,
SessionManagerException,
TokenException,
)
from ccx_upgrades_data_eng.config import get_settings, Settings
from ccx_upgrades_data_eng.config import Settings, get_settings
from ccx_upgrades_data_eng.inference import get_filled_inference_for_predictors
from ccx_upgrades_data_eng.models import (
ClustersList,
ClusterPrediction,
ClustersList,
MultiClusterUpgradeApiResponse,
UpgradeApiResponse,
)
Expand All @@ -26,14 +28,13 @@
perform_rhobs_request_multi_cluster,
)
from ccx_upgrades_data_eng.sentry import init_sentry
import ccx_upgrades_data_eng.metrics as metrics

from prometheus_fastapi_instrumentator import Instrumentator
from ccx_upgrades_data_eng.utils import get_retry_decorator

logger = logging.getLogger(__name__)

init_sentry(os.environ.get("SENTRY_DSN", None), None, os.environ.get("SENTRY_ENVIRONMENT", None))
init_sentry(
os.environ.get("SENTRY_DSN", None), None, os.environ.get("SENTRY_ENVIRONMENT", None)
)


def create_lifespan_handler(instrumentator: Instrumentator):
Expand Down Expand Up @@ -77,13 +78,13 @@ async def refresh_sso_token(request: Request, call_next) -> JSONResponse:
"""Middleware to ensure SSO token is refreshed before processing the request."""
try:
await get_session_and_refresh_token()
except SessionManagerException as ex:
except SessionManagerError as ex:
logger.error("Unable to initialize SSO session: %s", ex)
return JSONResponse(
"Unable to initialize SSO session",
status_code=status.HTTP_503_SERVICE_UNAVAILABLE,
)
except TokenException as ex:
except TokenError as ex:
logger.error("Unable to update SSO token: %s", ex)
return JSONResponse(
"Unable to update SSO token",
Expand All @@ -92,15 +93,22 @@ async def refresh_sso_token(request: Request, call_next) -> JSONResponse:
return await call_next(request)


@app.get("/cluster/{cluster_id}/upgrade-risks-prediction", response_model=UpgradeApiResponse)
async def upgrade_risks_prediction(cluster_id: UUID, settings: Settings = Depends(get_settings)):
@app.get(
"/cluster/{cluster_id}/upgrade-risks-prediction", response_model=UpgradeApiResponse
)
async def upgrade_risks_prediction(
cluster_id: UUID,
settings: Settings = Depends(get_settings), # noqa: B008
):
"""Return the predition of an upgrade failure given a set of alerts and focs."""
logger.info(f"Received cluster: {cluster_id}")
logger.debug("Getting predictors from RHOBS")
predictors, console_url = perform_rhobs_request(cluster_id)

if console_url is None or console_url == "":
return JSONResponse("No data for this cluster", status_code=status.HTTP_404_NOT_FOUND)
return JSONResponse(
"No data for this cluster", status_code=status.HTTP_404_NOT_FOUND
)

logger.debug("Getting inference result")
inference_result = get_filled_inference_for_predictors(predictors, console_url)
Expand All @@ -113,16 +121,19 @@ async def upgrade_risks_prediction(cluster_id: UUID, settings: Settings = Depend

@app.post("/upgrade-risks-prediction", response_model=MultiClusterUpgradeApiResponse)
async def upgrade_risks_multi_cluster_predictions(
clusters_list: ClustersList, settings: Settings = Depends(get_settings)
clusters_list: ClustersList,
settings: Settings = Depends(get_settings), # noqa: B008
):
"""Return the upgrade risks predictions for the provided clusters."""
logger.info("Received clusters list: %s", clusters_list)
logger.debug("Getting predictors from RHOBS or cache")
predictors_per_cluster = perform_rhobs_request_multi_cluster(clusters_list.clusters)

results = list()
results = []
for cluster, prediction in predictors_per_cluster.items():
inference_result = get_filled_inference_for_predictors(prediction[0], prediction[1])
inference_result = get_filled_inference_for_predictors(
prediction[0], prediction[1]
)
results.append(
ClusterPrediction(
cluster_id=str(cluster),
Expand Down
4 changes: 3 additions & 1 deletion ccx_upgrades_data_eng/metrics.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,9 @@ def update_ccx_upgrades_prediction_total(response: UpgradeApiResponse):

def update_ccx_upgrades_risks_total(response: UpgradeApiResponse):
"""Update CCX_UPGRADES_RISKS_TOTAL."""
CCX_UPGRADES_RISKS_TOTAL.labels("alerts").observe(len(response.upgrade_risks_predictors.alerts))
CCX_UPGRADES_RISKS_TOTAL.labels("alerts").observe(
len(response.upgrade_risks_predictors.alerts)
)
CCX_UPGRADES_RISKS_TOTAL.labels("operator_conditions").observe(
len(response.upgrade_risks_predictors.operator_conditions)
)
Expand Down
Loading
Loading