Merge remote-tracking branch 'origin/chore/improve_artifact_handling' into dev

Author: wp99cp

backend/migration/migrations/1778500427739-add-artifact-expiration-date.ts

@@ -0,0 +1,20 @@
+import { MigrationInterface, QueryRunner } from 'typeorm';
+
+export class AddArtifactExpirationDate1778500427739 implements MigrationInterface {
+    name = 'AddArtifactExpirationDate1778500427739';
+
+    public async up(queryRunner: QueryRunner): Promise<void> {
+        await queryRunner.query(
+            `ALTER TABLE "action" ADD "artifactExpirationDate" TIMESTAMP`,
+        );
+        await queryRunner.query(
+            `ALTER TYPE "public"."action_artifacts_enum" ADD VALUE '50'`,
+        );
+    }
+
+    public async down(queryRunner: QueryRunner): Promise<void> {
+        await queryRunner.query(
+            `ALTER TABLE "action" DROP COLUMN "artifactExpirationDate"`,
+        );
+    }
+}

backend/src/serialization/action.ts

@@ -6,6 +6,7 @@ import {
 } from '@kleinkram/api-dto';
 import { ActionEntity } from '@kleinkram/backend-common/entities/action/action.entity';
 import { WorkerEntity } from '@kleinkram/backend-common/entities/worker/worker.entity';
+import { ArtifactState } from '@kleinkram/shared';
 import { actionTemplateEntityToDto } from './action-template';
 import { missionEntityToDto, userEntityToDto } from './index';
 
@@ -52,9 +53,21 @@ export const actionEntityToDto = (action: ActionEntity): ActionDto => {
         runtime = (end - action.actionContainerStartedAt.getTime()) / 1000;
     }
 
+    let computedArtifactState = action.artifacts;
+    if (
+        action.artifactExpirationDate &&
+        action.artifactExpirationDate < new Date() &&
+        action.artifacts === ArtifactState.UPLOADED
+    ) {
+        computedArtifactState = ArtifactState.EXPIRED;
+    }
+
     return {
-        artifactUrl: action.artifact_path ?? '',
-        artifacts: action.artifacts,
+        artifactUrl:
+            computedArtifactState === ArtifactState.EXPIRED
+                ? ''
+                : (action.artifact_path ?? ''),
+        artifacts: computedArtifactState,
         artifactSize: action.artifact_size,
         artifactFiles: action.artifact_files,
         // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition

backend/src/services/action.service.ts

@@ -182,7 +182,7 @@ export class ActionService {
 
         const dto = actionEntityToDto(action);
 
-        if (action.artifacts === ArtifactState.UPLOADED) {
+        if (dto.artifacts === ArtifactState.UPLOADED) {
             dto.artifactUrl = await this.generateArtifactUrl(
                 dto.artifactUrl,
                 action,

cli/kleinkram/api/deser.py

@@ -14,6 +14,7 @@
 
 from kleinkram.errors import ParsingError
 from kleinkram.models import ActionTemplate
+from kleinkram.models import ArtifactState
 from kleinkram.models import Execution
 from kleinkram.models import File
 from kleinkram.models import FileState
@@ -81,6 +82,8 @@ class ExecutionObjectKeys(str, Enum):
     UPDATED_AT = "updatedAt"
     LOGS = "logs"
     ARTIFACT_URL = "artifactUrl"
+    ARTIFACT_STATE = "artifacts"
+    ARTIFACT_SIZE = "artifactSize"
 
 
 class TemplateObjectKeys(str, Enum):
@@ -269,6 +272,9 @@ def _parse_execution(execution_object: ExecutionObject) -> Execution:
         state = execution_object[ExecutionObjectKeys.STATE]
         state_cause = execution_object[ExecutionObjectKeys.STATE_CAUSE]
         artifact_url = execution_object.get(ExecutionObjectKeys.ARTIFACT_URL)
+        raw_state = execution_object.get(ExecutionObjectKeys.ARTIFACT_STATE)
+        artifact_state = ArtifactState(raw_state) if raw_state is not None else None
+        artifact_size = execution_object.get(ExecutionObjectKeys.ARTIFACT_SIZE)
         created_at = _parse_datetime(execution_object[ExecutionObjectKeys.CREATED_AT])
         updated_at = (
             _parse_datetime(execution_object[ExecutionObjectKeys.UPDATED_AT])
@@ -307,6 +313,8 @@ def _parse_execution(execution_object: ExecutionObject) -> Execution:
         state=state,
         state_cause=state_cause,
         artifact_url=artifact_url,
+        artifact_state=artifact_state,
+        artifact_size=artifact_size,
         created_at=created_at,
         updated_at=updated_at,
         mission_id=mission_id,

cli/kleinkram/cli/_executions.py

@@ -257,7 +257,12 @@ def logs(
 @executions_typer.command(name="download", help=DOWNLOAD_HELP)
 def download_artifacts(
     execution: str = typer.Argument(..., metavar="EXECUTION_ID", help="The ID of the execution to download artifacts for."),
-    output: Optional[str] = typer.Option(None, "--output", "-o", help="Path or filename to save the artifacts to."),
+    output_dir: Optional[str] = typer.Option(
+        None, "--output-dir", "-o", help="Directory to save the artifacts to (defaults to current directory)."
+    ),
+    filename: Optional[str] = typer.Option(
+        None, "--filename", "-f", help="Filename to save the artifact as (must end in .tar.gz)."
+    ),
     extract: bool = typer.Option(
         False,
         "--extract",
@@ -279,7 +284,8 @@ def download_artifacts(
         kleinkram.core.download_artifact(
             client=client,
             execution_id=execution_id,
-            output=output,
+            output_dir=output_dir,
+            filename=filename,
             extract=extract,
             verbose=get_shared_state().verbose,
         )

cli/kleinkram/core.py

@@ -29,7 +29,6 @@
 from uuid import UUID
 
 import httpx
-import requests
 from rich.console import Console
 from tqdm import tqdm
 
@@ -45,6 +44,7 @@
 from kleinkram.errors import InvalidFileQuery
 from kleinkram.errors import MissionNotFound
 from kleinkram.errors import TemplateNotFound
+from kleinkram.models import ArtifactState
 from kleinkram.models import FileState
 from kleinkram.models import FileVerificationStatus
 from kleinkram.printing import files_to_table
@@ -65,7 +65,8 @@ def download_artifact(
     *,
     client: AuthenticatedClient,
     execution_id: UUID,
-    output: Optional[str] = None,
+    output_dir: Optional[str] = None,
+    filename: Optional[str] = None,
     extract: bool = False,
     verbose: bool = False,
 ) -> str:
@@ -74,7 +75,8 @@ def download_artifact(
 
     Args:
         execution_id: The ID of the execution to download artifacts for.
-        output: Path or filename to save the artifacts to.
+        output_dir: Directory to save the artifacts to (optional).
+        filename: Custom filename for the downloaded artifact (optional, must end with .tar.gz).
         extract: Automatically extract the archive after downloading.
         verbose: Print progress and extraction info.
 
@@ -85,52 +87,41 @@ def download_artifact(
     # Fetch Execution Details
     execution = kleinkram.api.routes.get_execution(client, execution_id=execution_id)
 
+    if execution.artifact_state == ArtifactState.EXPIRED:
+        raise ValueError(f"Artifacts for execution {execution_id} have expired and therefore been deleted.")
+
     if not execution.artifact_url:
+        raise ValueError(f"No artifacts found for execution {execution_id}. The execution might not be finished yet.")
+
+    if not execution.artifact_size:
         raise ValueError(
-            f"No artifacts found for execution {execution_id}. The execution might not be finished or artifacts expired."
+            f"Artifact size is missing, empty, or corrupted for execution {execution_id}, cannot proceed with download."
         )
 
     if verbose:
         print(f"Downloading artifacts for execution {execution_id}...")
 
-    # Grab the headers to determine filename and size
-    try:
-        with requests.get(execution.artifact_url, stream=True) as r:
-            r.raise_for_status()
-            headers = r.headers.copy()
-    except requests.exceptions.RequestException as e:
-        raise ValueError(f"Error fetching artifact headers: {e}")
-
-    # Determine Filename
-    filename = output
-    if not filename:
-        filename = kleinkram.api.file_transfer._get_filename_from_cd(headers.get("content-disposition"))
+    if filename:
+        if not filename.endswith(".tar.gz"):
+            raise ValueError(f"Filename must end with .tar.gz, got: {filename}")
 
     if not filename:
-        filename = f"{execution_id}.tar.gz"
+        filename = f"{execution.template_name}-{execution_id}.tar.gz"
 
-    # If output is a directory, join with filename
-    if output and os.path.isdir(output):
-        filename = os.path.join(
-            output,
-            kleinkram.api.file_transfer._get_filename_from_cd(headers.get("content-disposition")) or f"{execution_id}.tar.gz",
-        )
+    if output_dir and not os.path.isdir(output_dir):
+        raise ValueError(f"Output directory {output_dir} does not exist or is not a directory.")
 
-    total_length_raw = headers.get("content-length")
-    if total_length_raw is None:
-        raise ValueError(
-            f"Cannot determine artifact size for execution {execution_id}: "
-            "the server did not return a content-length header."
-        )
-    total_length = int(total_length_raw)
+    filepath = Path(os.path.join(output_dir, filename) if output_dir else filename)
 
-    filepath = Path(filename)
+    if verbose:
+        print(f"Artifact size: {execution.artifact_size} bytes")
+        print(f"Saving artifact to: {filepath}...")
 
     # Download the file using _url_download
     kleinkram.api.file_transfer._url_download(
         url=execution.artifact_url,
         path=filepath,
-        size=total_length,
+        size=execution.artifact_size,
         overwrite=True,  # overwrite if we run the CLI command directly
         verbose=verbose,
     )

cli/kleinkram/models.py

@@ -4,6 +4,7 @@
 from dataclasses import field
 from datetime import datetime
 from enum import Enum
+from enum import IntEnum
 from typing import Dict
 from typing import List
 from uuid import UUID
@@ -85,6 +86,14 @@ class ExecutionStatus(str, Enum):
     CANCELLED = "Cancelled"
 
 
+class ArtifactState(IntEnum):
+    AWAITING_ACTION = 10
+    UPLOADING = 20
+    UPLOADED = 30
+    ERROR = 40
+    EXPIRED = 50
+
+
 @dataclass(frozen=True)
 class LogEntry:
     timestamp: datetime
@@ -98,6 +107,8 @@ class Execution:
     state: str
     state_cause: str | None
     artifact_url: str | None
+    artifact_state: ArtifactState | None
+    artifact_size: int | None
     created_at: datetime
     updated_at: datetime | None
     project_name: str

cli/kleinkram/wrappers.py

@@ -161,7 +161,8 @@ def download(
 
 def download_artifact(
     execution_id: IdLike,
-    output: Optional[PathLike] = None,
+    output_dir: Optional[PathLike] = None,
+    filename: Optional[str] = None,
     extract: bool = False,
     verbose: bool = False,
     *,
@@ -172,7 +173,8 @@ def download_artifact(
 
     Args:
         execution_id: The ID of the execution to download artifacts for.
-        output: Path or filename to save the artifacts to.
+        output_dir: Directory to save the artifacts to (optional, defaults to current directory).
+        filename: Filename to save the artifact as (optional, must end in .tar.gz).
         extract: Automatically extract the archive after downloading.
         verbose: Print progress and extraction info.
 
@@ -183,7 +185,8 @@ def download_artifact(
     return kleinkram.core.download_artifact(
         client=client,
         execution_id=parse_uuid_like(execution_id),
-        output=str(parse_path_like(output)) if output else None,
+        output_dir=str(output_dir) if output_dir else None,
+        filename=filename,
         extract=extract,
         verbose=verbose,
     )

frontend/src/components/actions/action-details-execution-tab.vue

@@ -116,12 +116,8 @@
                         unelevated
                         class="bg-button-secondary text-on-color"
                         icon="sym_o_download"
-                        :disable="isExpired"
                         @click="openArtifact"
                     >
-                        <q-tooltip v-if="isExpired">
-                            Artifacts are only available for 3 months.
-                        </q-tooltip>
                     </q-btn>
                     <div v-else class="text-grey-7">
                         {{ artifactStateText }}
@@ -431,6 +427,9 @@ const artifactStateText = computed(() => {
     }
 
     switch (props.action.artifacts) {
+        case ArtifactState.EXPIRED: {
+            return 'Artifacts expired and deleted';
+        }
         case ArtifactState.UPLOADING: {
             return 'Uploading...';
         }
@@ -459,16 +458,6 @@ const showArtifactTree = computed(() => {
     );
 });
 
-const isExpired = computed(() => {
-    // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition
-    if (!props.action.createdAt) return false;
-    const created = new Date(props.action.createdAt);
-    const now = new Date();
-    // 3 months ago
-    const threeMonthsAgo = new Date(now.setMonth(now.getMonth() - 3));
-    return created < threeMonthsAgo;
-});
-
 const openArtifact = (): void => {
     if (props.action.artifactUrl) {
         window.open(props.action.artifactUrl, '_blank');

packages/backend-common/src/entities/action/action.entity.ts

@@ -116,6 +116,9 @@ export class ActionEntity extends BaseEntity {
     // eslint-disable-next-line @typescript-eslint/naming-convention
     artifact_files?: string[];
 
+    @Column({ nullable: true })
+    artifactExpirationDate?: Date;
+
     @OneToOne(() => ApiKeyEntity, (apikey) => apikey.action)
     @JoinColumn()
     key?: ApiKeyEntity;