diff --git a/.github/workflows/outdated_dependencies.yaml b/.github/workflows/outdated_dependencies.yaml new file mode 100644 index 00000000..13b855d5 --- /dev/null +++ b/.github/workflows/outdated_dependencies.yaml @@ -0,0 +1,23 @@ +name: List outdated dependencies + +on: + - push + - pull_request + +jobs: + list_outdated_dependencies: + runs-on: ubuntu-latest + permissions: + contents: read + pull-requests: read + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-python@v5 + with: + python-version: '3.11' + - name: Check Python version + run: python --version + - name: PDM installation + run: pip install --user pdm + - name: List outdated dependencies + run: pdm outdated diff --git a/.github/workflows/pydocstyle.yaml b/.github/workflows/pydocstyle.yaml new file mode 100644 index 00000000..101fce9e --- /dev/null +++ b/.github/workflows/pydocstyle.yaml @@ -0,0 +1,23 @@ +name: Pydocstyle + +on: + - push + - pull_request + +jobs: + pydocstyle: + runs-on: ubuntu-latest + permissions: + contents: read + pull-requests: read + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-python@v5 + with: + python-version: '3.11' + - name: Python version + run: python --version + - name: Pydocstyle install + run: pip install --user pydocstyle + - name: Python docstring checks + run: pydocstyle -v . diff --git a/.github/workflows/radon.yaml b/.github/workflows/radon.yaml new file mode 100644 index 00000000..f21e9e87 --- /dev/null +++ b/.github/workflows/radon.yaml @@ -0,0 +1,20 @@ +name: Radon + +on: + - push + - pull_request + +jobs: + radon: + runs-on: ubuntu-latest + permissions: + contents: read + pull-requests: read + name: "radon" + steps: + - uses: actions/checkout@v3 + - uses: davidslusser/actions_python_radon@v1.0.0 + with: + src: "src" + min: "A" + grade: "B" diff --git a/.github/workflows/shellcheck.yaml b/.github/workflows/shellcheck.yaml new file mode 100644 index 00000000..fa3cea8e --- /dev/null +++ b/.github/workflows/shellcheck.yaml @@ -0,0 +1,16 @@ +name: Shell check + +on: + - push + - pull_request + +jobs: + shellcheck: + runs-on: ubuntu-latest + permissions: + contents: read + pull-requests: read + steps: + - uses: actions/checkout@v4 + - name: Shell check + run: make shellcheck diff --git a/Makefile b/Makefile index 48714f3f..a1a49cf4 100644 --- a/Makefile +++ b/Makefile @@ -38,3 +38,8 @@ docs/config.png: docs/config.puml ## Generate an image with configuration graph mv classes.png config.png && \ popd +shellcheck: ## Run shellcheck + wget -qO- "https://github.com/koalaman/shellcheck/releases/download/stable/shellcheck-stable.linux.x86_64.tar.xz" | tar -xJv \ + shellcheck --version + shellcheck -- */*.sh + diff --git a/scripts/codecov.sh b/scripts/codecov.sh new file mode 100755 index 00000000..52e80b68 --- /dev/null +++ b/scripts/codecov.sh @@ -0,0 +1,53 @@ +#!/usr/bin/env bash + +set -o nounset +set -o pipefail +set -x + +CI_SERVER_URL=https://prow.svc.ci.openshift.org/view/gcs/origin-ci-test +COVER_PROFILE=${COVER_PROFILE:-"$1"} +JOB_TYPE=${JOB_TYPE:-"local"} + +# Configure the git refs and job link based on how the job was triggered via prow +if [[ "${JOB_TYPE}" == "presubmit" ]]; then + echo "detected PR code coverage job for #${PULL_NUMBER}" + REF_FLAGS="-P ${PULL_NUMBER} -C ${PULL_PULL_SHA}" + JOB_LINK="${CI_SERVER_URL}/pr-logs/pull/${REPO_OWNER}_${REPO_NAME}/${PULL_NUMBER}/${JOB_NAME}/${BUILD_ID}" +elif [[ "${JOB_TYPE}" == "batch" ]] || [[ "${JOB_TYPE}" == "postsubmit" ]]; then + echo "detected branch code coverage job for ${PULL_BASE_REF}" + REF_FLAGS="-B ${PULL_BASE_REF} -C ${PULL_BASE_SHA}" + JOB_LINK="${CI_SERVER_URL}/logs/${JOB_NAME}/${BUILD_ID}" +elif [[ "${JOB_TYPE}" == "local" ]]; then + echo "coverage report available at ${COVER_PROFILE}" + exit 0 +else + echo "${JOB_TYPE} jobs not supported" >&2 + exit 1 +fi + +# Configure certain internal codecov variables with values from prow. +export CI_BUILD_URL="${JOB_LINK}" +export CI_BUILD_ID="${JOB_NAME}" +export CI_JOB_ID="${BUILD_ID}" + +if [[ "${JOB_TYPE}" != "local" ]]; then + if [[ -z "${ARTIFACT_DIR:-}" ]] || [[ ! -d "${ARTIFACT_DIR}" ]] || [[ ! -w "${ARTIFACT_DIR}" ]]; then + # shellcheck disable=SC2016 + echo '${ARTIFACT_DIR} must be set for non-local jobs, and must point to a writable directory' >&2 + exit 1 + fi + curl -sS https://codecov.io/bash -o "${ARTIFACT_DIR}/codecov.sh" + bash <(cat "${ARTIFACT_DIR}/codecov.sh") -Z -K -f "${COVER_PROFILE}" -r "${REPO_OWNER}/${REPO_NAME}" "${REF_FLAGS}" + # shellcheck disable=SC2181 + if [ $? -ne 0 ]; then + echo "Failed uploading coverage report from a non local environment. Exiting gracefully with status code 0." + exit 0 + fi +else + bash <(curl -s https://codecov.io/bash) -Z -K -f "${COVER_PROFILE}" -r "${REPO_OWNER}/${REPO_NAME}" "${REF_FLAGS}" + # shellcheck disable=SC2181 + if [ $? -ne 0 ]; then + echo "Failed uploading coverage report from local environment. Exiting gracefully with status code 0." + exit 0 + fi +fi diff --git a/src/app/endpoints/config.py b/src/app/endpoints/config.py index 19ab1f2d..a5c280cd 100644 --- a/src/app/endpoints/config.py +++ b/src/app/endpoints/config.py @@ -22,4 +22,5 @@ @router.get("/config", responses=get_config_responses) def config_endpoint_handler(request: Request) -> Configuration: + """Handle requests to the /config endpoint.""" return configuration.configuration diff --git a/src/app/endpoints/info.py b/src/app/endpoints/info.py index 3197477f..ac0a289d 100644 --- a/src/app/endpoints/info.py +++ b/src/app/endpoints/info.py @@ -22,4 +22,5 @@ @router.get("/info", responses=get_into_responses) def info_endpoint_handler(request: Request) -> InfoResponse: + """Handle request to the /info endpoint.""" return InfoResponse(name="foo", version=__version__) diff --git a/src/app/endpoints/models.py b/src/app/endpoints/models.py index d9b9d40d..4b4ed830 100644 --- a/src/app/endpoints/models.py +++ b/src/app/endpoints/models.py @@ -41,6 +41,7 @@ @router.get("/models", responses=models_responses) def models_endpoint_handler(request: Request) -> ModelsResponse: + """Handle requests to the /models endpoint.""" llama_stack_config = configuration.llama_stack_configuration logger.info("LLama stack config: %s", llama_stack_config) diff --git a/src/app/endpoints/query.py b/src/app/endpoints/query.py index d55cc5d0..a284a1cb 100644 --- a/src/app/endpoints/query.py +++ b/src/app/endpoints/query.py @@ -32,6 +32,7 @@ def query_endpoint_handler( request: Request, query_request: QueryRequest ) -> QueryResponse: + """Handle request to the /query endpoint.""" llama_stack_config = configuration.llama_stack_configuration logger.info("LLama stack config: %s", llama_stack_config) client = get_llama_stack_client(llama_stack_config) @@ -84,7 +85,7 @@ def select_model_id(client: LlamaStackClient, query_request: QueryRequest) -> st def retrieve_response( client: LlamaStackClient, model_id: str, query_request: QueryRequest ) -> str: - + """Retrieve response from LLMs and agents.""" available_shields = [shield.identifier for shield in client.shields.list()] if not available_shields: logger.info("No available shields. Disabling safety") @@ -124,6 +125,7 @@ def retrieve_response( def validate_attachments_metadata(attachments: list[Attachment]) -> None: """Validate the attachments metadata provided in the request. + Raises HTTPException if any attachment has an improper type or content type. """ for attachment in attachments: diff --git a/src/app/endpoints/root.py b/src/app/endpoints/root.py index 5e1c8cee..a8852531 100644 --- a/src/app/endpoints/root.py +++ b/src/app/endpoints/root.py @@ -27,5 +27,6 @@ @router.get("/", response_class=HTMLResponse) def root_endpoint_handler(request: Request) -> HTMLResponse: + """Handle request to the / endpoint.""" logger.info("Serving index page") return HTMLResponse(index_page) diff --git a/src/app/main.py b/src/app/main.py index 0b1f6cf0..dea878ea 100644 --- a/src/app/main.py +++ b/src/app/main.py @@ -1,3 +1,5 @@ +"""Definition of FastAPI based web service.""" + from fastapi import FastAPI from app import routers import version @@ -25,4 +27,5 @@ @app.on_event("startup") async def startup_event() -> None: + """Perform logger setup on service startup.""" get_logger("app.endpoints.handlers") diff --git a/src/client.py b/src/client.py index 77073dea..4e620a71 100644 --- a/src/client.py +++ b/src/client.py @@ -12,6 +12,7 @@ def get_llama_stack_client( llama_stack_config: LLamaStackConfiguration, ) -> LlamaStackClient: + """Retrieve Llama stack client according to configuration.""" if llama_stack_config.use_as_library_client is True: if llama_stack_config.library_client_config_path is not None: logger.info("Using Llama stack as library client") diff --git a/src/configuration.py b/src/configuration.py index c0da10ec..1f65aad2 100644 --- a/src/configuration.py +++ b/src/configuration.py @@ -1,3 +1,5 @@ +"""Configuration loader.""" + import yaml import logging @@ -8,6 +10,8 @@ class AppConfig: + """Singleton class to load and store the configuration.""" + _instance = None def __new__(cls, *args: Any, **kwargs: Any) -> "AppConfig": @@ -28,10 +32,12 @@ def load_configuration(self, filename: str) -> None: self.init_from_dict(config_dict) def init_from_dict(self, config_dict: dict[Any, Any]) -> None: + """Initialize configuration from a dictionary.""" self._configuration = Configuration(**config_dict) @property def configuration(self) -> Configuration: + """Return the whole configuration.""" assert ( self._configuration is not None ), "logic error: configuration is not loaded" @@ -39,6 +45,7 @@ def configuration(self) -> Configuration: @property def llama_stack_configuration(self) -> LLamaStackConfiguration: + """Return Llama stack configuration.""" assert ( self._configuration is not None ), "logic error: configuration is not loaded" diff --git a/src/constants.py b/src/constants.py index 5699962e..e52b06c3 100644 --- a/src/constants.py +++ b/src/constants.py @@ -1,3 +1,5 @@ +"""Constants used in business logic.""" + UNABLE_TO_PROCESS_RESPONSE = "Unable to process this request" # Supported attachment types diff --git a/src/lightspeed-stack.py b/src/lightspeed-stack.py index 1b7d4bf6..9cd8c7b7 100644 --- a/src/lightspeed-stack.py +++ b/src/lightspeed-stack.py @@ -18,6 +18,7 @@ def create_argument_parser() -> ArgumentParser: + """Create and configure argument parser object.""" parser = ArgumentParser() parser.add_argument( "-v", @@ -45,6 +46,7 @@ def dump_configuration(configuration: Configuration) -> None: def main() -> None: + """Entry point to the web service.""" logger.info("Lightspeed stack startup") parser = create_argument_parser() args = parser.parse_args() diff --git a/src/log.py b/src/log.py index e8290f67..de36b709 100644 --- a/src/log.py +++ b/src/log.py @@ -1,8 +1,11 @@ +"""Log utilities.""" + import logging from rich.logging import RichHandler def get_logger(name: str) -> logging.Logger: + """Retrieve logger with the provided name.""" logger = logging.getLogger(name) logger.setLevel(logging.DEBUG) logger.handlers = [RichHandler()] diff --git a/src/models/config.py b/src/models/config.py index b8dc15db..00a672c7 100644 --- a/src/models/config.py +++ b/src/models/config.py @@ -1,3 +1,5 @@ +"""Model with service configuration.""" + from pydantic import BaseModel, model_validator from typing import Optional @@ -16,6 +18,7 @@ class ServiceConfiguration(BaseModel): @model_validator(mode="after") def check_service_configuration(self) -> Self: + """Check service configuration.""" if self.port <= 0: raise ValueError("Port value should not be negative") if self.port > 65535: @@ -35,6 +38,7 @@ class LLamaStackConfiguration(BaseModel): @model_validator(mode="after") def check_llama_stack_model(self) -> Self: + """Check Llama stack configuration.""" if self.url is None: if self.use_as_library_client is None: raise ValueError( diff --git a/src/models/requests.py b/src/models/requests.py index c71f333b..a3c78d2a 100644 --- a/src/models/requests.py +++ b/src/models/requests.py @@ -1,3 +1,5 @@ +"""Model for service requests.""" + from pydantic import BaseModel, model_validator from llama_stack_client.types.agents.turn_create_params import Document from typing import Optional, Self @@ -110,7 +112,7 @@ class QueryRequest(BaseModel): } def get_documents(self) -> list[Document]: - """Returns the list of documents from the attachments.""" + """Return the list of documents from the attachments.""" if not self.attachments: return [] return [ diff --git a/src/models/responses.py b/src/models/responses.py index b1398269..f34d417a 100644 --- a/src/models/responses.py +++ b/src/models/responses.py @@ -1,3 +1,5 @@ +"""Models for service responses.""" + from pydantic import BaseModel from typing import Any, Optional