[DOP-21268] - minor fixes

Author: maxim-lixakov

syncmaster/backend/api/v1/auth.py

@@ -1,25 +1,23 @@
 # SPDX-FileCopyrightText: 2023-2024 MTS PJSC
 # SPDX-License-Identifier: Apache-2.0
-from typing import TYPE_CHECKING, Annotated
+from typing import Annotated
 
 from fastapi import APIRouter, Depends, HTTPException, Request
 from fastapi.responses import RedirectResponse
 from fastapi.security import OAuth2PasswordRequestForm
 
 from syncmaster.backend.dependencies import Stub
-from syncmaster.backend.providers.auth import AuthProvider
+from syncmaster.backend.providers.auth import (
+    AuthProvider,
+    DummyAuthProvider,
+    KeycloakAuthProvider,
+)
 from syncmaster.backend.utils.state import validate_state
 from syncmaster.errors.registration import get_error_responses
 from syncmaster.errors.schemas.invalid_request import InvalidRequestSchema
 from syncmaster.errors.schemas.not_authorized import NotAuthorizedSchema
 from syncmaster.schemas.v1.auth import AuthTokenSchema
 
-if TYPE_CHECKING:
-    from syncmaster.backend.providers.auth import (
-        DummyAuthProvider,
-        KeycloakAuthProvider,
-    )
-
 router = APIRouter(
     prefix="/auth",
     tags=["Auth"],
@@ -29,7 +27,7 @@
 
 @router.post("/token")
 async def token(
-    auth_provider: Annotated["DummyAuthProvider", Depends(Stub(AuthProvider))],
+    auth_provider: Annotated[DummyAuthProvider, Depends(Stub(AuthProvider))],
     form_data: OAuth2PasswordRequestForm = Depends(),
 ) -> AuthTokenSchema:
     token = await auth_provider.get_token_password_grant(
@@ -48,7 +46,7 @@ async def auth_callback(
     request: Request,
     code: str,
     state: str,
-    auth_provider: Annotated["KeycloakAuthProvider", Depends(Stub(AuthProvider))],
+    auth_provider: Annotated[KeycloakAuthProvider, Depends(Stub(AuthProvider))],
 ):
     original_redirect_url = validate_state(state)
     if not original_redirect_url:

syncmaster/backend/api/v1/runs.py

@@ -22,7 +22,9 @@
     RunPageSchema,
 )
 from syncmaster.worker.config import celery
-from syncmaster.worker.settings import worker_settings
+
+# TODO: remove global import of WorkerSettings
+from syncmaster.worker.settings import WorkerSettings as Settings
 
 router = APIRouter(tags=["Runs"], responses=get_error_responses())
 
@@ -117,7 +119,7 @@ async def start_run(
             type=RunType.MANUAL,
         )
 
-        log_url = Template(worker_settings.LOG_URL_TEMPLATE).render(
+        log_url = Template(Settings().LOG_URL_TEMPLATE).render(
             run=run,
             correlation_id=correlation_id.get(),
         )

syncmaster/backend/providers/auth/keycloak_provider.py

@@ -105,6 +105,8 @@ async def get_current_user(self, access_token: str, *args, **kwargs) -> Any:
                 log.debug("Failed to refresh access token: %s", e)
                 self.redirect_to_auth(request.url.path)
 
+        # these names are hardcoded in keycloak:
+        # https://github.com/keycloak/keycloak/blob/3ca3a4ad349b4d457f6829eaf2ae05f1e01408be/core/src/main/java/org/keycloak/representations/IDToken.java
         user_id = token_info.get("sub")
         login = token_info.get("preferred_username")
         email = token_info.get("email")
@@ -133,9 +135,6 @@ async def refresh_access_token(self, refresh_token: str) -> dict[str, Any]:
         new_tokens = self.keycloak_openid.refresh_token(refresh_token)
         return new_tokens
 
-    async def get_user_info(self, access_token: str) -> dict[str, Any]:
-        return self.keycloak_openid.userinfo(access_token)
-
     def redirect_to_auth(self, path: str) -> None:
         state = generate_state(path)
         auth_url = self.keycloak_openid.auth_url(

syncmaster/backend/services/get_user.py

@@ -21,7 +21,7 @@ def get_user(
     async def wrapper(
         request: Request,
         auth_provider: Annotated[AuthProvider, Depends(Stub(AuthProvider))],
-        access_token: Annotated[str, Depends(oauth_schema)],
+        access_token: Annotated[str | None, Depends(oauth_schema)],
     ) -> User:
         # keycloak provider patches session and store access_token in cookie,
         # when dummy auth stores it in "Authorization" header

syncmaster/worker/base.py

@@ -3,12 +3,13 @@
 from celery import Task
 from sqlalchemy import create_engine
 
-from syncmaster.worker.settings import worker_settings
+# TODO: remove global import of WorkerSettings
+from syncmaster.worker.settings import WorkerSettings as Settings
 
 
 class WorkerTask(Task):
     def __init__(self) -> None:
-        self.settings = worker_settings
+        self.settings = Settings()
         self.engine = create_engine(
             url=self.settings.database.sync_url,
         )

syncmaster/worker/config.py

@@ -3,8 +3,11 @@
 from celery import Celery
 
 from syncmaster.worker.base import WorkerTask
-from syncmaster.worker.settings import worker_settings
 
+# TODO: remove global import of WorkerSettings
+from syncmaster.worker.settings import WorkerSettings as Settings
+
+worker_settings = Settings()
 celery = Celery(
     __name__,
     broker=worker_settings.broker.url,

syncmaster/worker/controller.py

@@ -25,7 +25,9 @@
 from syncmaster.worker.handlers.db.postgres import PostgresHandler
 from syncmaster.worker.handlers.file.hdfs import HDFSHandler
 from syncmaster.worker.handlers.file.s3 import S3Handler
-from syncmaster.worker.settings import worker_settings
+
+# TODO: remove global import of WorkerSettings
+from syncmaster.worker.settings import WorkerSettings as Settings
 
 logger = logging.getLogger(__name__)
 
@@ -84,7 +86,7 @@ def __init__(
         )
 
     def perform_transfer(self) -> None:
-        spark = worker_settings.CREATE_SPARK_SESSION_FUNCTION(
+        spark = Settings().CREATE_SPARK_SESSION_FUNCTION(
             run=self.run,
             source=self.source_handler.connection_dto,
             target=self.target_handler.connection_dto,

syncmaster/worker/settings/__init__.py

@@ -20,6 +20,3 @@ class WorkerSettings(SyncmasterSettings):
     LOG_URL_TEMPLATE: str = ""
 
     CREATE_SPARK_SESSION_FUNCTION: ImportString = "syncmaster.worker.spark.get_worker_spark_session"
-
-
-worker_settings = WorkerSettings()

syncmaster/worker/transfer.py

@@ -18,11 +18,12 @@
 from syncmaster.worker.base import WorkerTask
 from syncmaster.worker.config import celery
 from syncmaster.worker.controller import TransferController
-from syncmaster.worker.settings import worker_settings
+from syncmaster.worker.settings import WorkerSettings as WorkerSettings
 
 logger = get_task_logger(__name__)
 
-CORRELATION_CELERY_HEADER_ID = worker_settings.CORRELATION_CELERY_HEADER_ID
+# TODO: remove global import of WorkerSettings
+CORRELATION_CELERY_HEADER_ID = WorkerSettings().CORRELATION_CELERY_HEADER_ID
 
 
 @celery.task(name="run_transfer_task", bind=True, track_started=True)
@@ -85,7 +86,7 @@ def run_transfer(session: Session, run_id: int, settings: Settings):
 
 @after_setup_logger.connect
 def setup_loggers(*args, **kwargs):
-    setup_logging(worker_settings.logging.get_log_config_path())
+    setup_logging(Settings().logging.get_log_config_path())
 
 
 @before_task_publish.connect()

tests/test_unit/test_runs/test_create_run.py

@@ -6,7 +6,9 @@
 from sqlalchemy.ext.asyncio import AsyncSession
 
 from syncmaster.db.models import Run, RunType, Status
-from syncmaster.worker.settings import worker_settings
+
+# TODO: remove global import of WorkerSettings
+from syncmaster.worker.settings import WorkerSettings as Settings
 from tests.mocks import MockGroup, MockTransfer, MockUser, UserTestRoles
 
 pytestmark = [pytest.mark.asyncio, pytest.mark.backend]
@@ -140,9 +142,7 @@ async def test_superuser_can_create_run(
     mocker,
 ) -> None:
     # Arrange
-    worker_settings.LOG_URL_TEMPLATE = (
-        "https://grafana.example.com?correlation_id={{ correlation_id }}&run_id={{ run.id }}"
-    )
+    Settings().LOG_URL_TEMPLATE = "https://grafana.example.com?correlation_id={{ correlation_id }}&run_id={{ run.id }}"
     mock_send_task = mocker.patch("syncmaster.worker.config.celery.send_task")
     mock_to_thread = mocker.patch("asyncio.to_thread", new_callable=AsyncMock)