Merge pull request #167 from fgcz/main

bfabric_app_runner 0.0.20

Author: leoschwarz

bfabric/docs/changelog.md

@@ -10,6 +10,24 @@ Versioning currently follows `X.Y.Z` where
 
 ## \[Unreleased\]
 
+## \[1.13.23\] - 2025-03-25
+
+### Fixed
+
+- Unsuccessful deletions are detected by checking the B-Fabric response.
+- Handle problematic characters in `Workunit.store_output_folder`.
+- `BfabricRequestError` did not properly subclass RuntimeError.
+
+### Changed
+
+- `WorkunitDefinition` uses `PathSafeStr` to normalize app and workunit names.
+- Internal: `ResultContainer` has no optional constructor arguments anymore to avoid confusion.
+
+### Added
+
+- Generic functionality in `bfabric.utils.path_safe_name` to validate names for use in paths.
+- `bfabric.entities.Dataset.{write_parquet, get_parquet}` methods for writing parquet
+
 ## \[1.13.22\] - 2025-02-19
 
 ### Fixed

bfabric/pyproject.toml

@@ -5,7 +5,7 @@ build-backend = "hatchling.build"
 [project]
 name = "bfabric"
 description = "Python client for the B-Fabric API"
-version = "1.13.22"
+version = "1.13.23"
 license = { text = "GPL-3.0" }
 authors = [
     { name = "Christian Panse", email = "cp@fgcz.ethz.ch" },
@@ -46,7 +46,7 @@ dev = [
 ]
 doc = ["mkdocs", "mkdocs-material", "mkdocstrings[python]"]
 test = ["pytest", "pytest-mock", "logot[pytest,loguru]"]
-typing = ["mypy", "types-requests", "lxml-stubs", "pandas-stubs", "types-python-dateutil"]
+typing = ["mypy", "types-requests", "lxml-stubs", "pandas-stubs", "types-python-dateutil", "types-PyYAML"]
 
 [project.urls]
 Homepage = "https://github.com/fgcz/bfabricPy"

bfabric/src/bfabric/engine/engine_suds.py

@@ -9,8 +9,9 @@
 
 from bfabric.engine.response_format_suds import suds_asdict_recursive
 from bfabric.errors import BfabricRequestError, get_response_errors
-from bfabric.results.result_container import ResultContainer
+from bfabric.results.response_delete import ResponseDelete
 from bfabric.results.response_format_dict import clean_result
+from bfabric.results.result_container import ResultContainer
 
 if TYPE_CHECKING:
     from suds.serviceproxy import ServiceProxy
@@ -46,13 +47,13 @@ def read(
         query = copy.deepcopy(obj)
         query["includedeletableupdateable"] = include_deletable_and_updatable_fields
 
-        full_query = dict(
-            login=auth.login,
-            page=page,
-            password=auth.password.get_secret_value(),
-            query=query,
-            idonly=return_id_only,
-        )
+        full_query = {
+            "login": auth.login,
+            "page": page,
+            "password": auth.password.get_secret_value(),
+            "query": query,
+            "idonly": return_id_only,
+        }
         service = self._get_suds_service(endpoint)
         response = service.read(full_query)
         return self._convert_results(response=response, endpoint=endpoint)
@@ -80,14 +81,11 @@ def delete(self, endpoint: str, id: int | list[int], auth: BfabricAuth) -> Resul
         :param auth: the authentication handle of the user performing the request
         """
         if isinstance(id, list) and len(id) == 0:
-            print("Warning, attempted to delete an empty list, ignoring")
-            # TODO maybe use error here (and make sure it's consistent)
-            return ResultContainer([], total_pages_api=0)
-
+            return ResponseDelete.from_empty_request()
         query = {"login": auth.login, "password": auth.password.get_secret_value(), "id": id}
         service = self._get_suds_service(endpoint)
         response = service.delete(query)
-        return self._convert_results(response=response, endpoint=endpoint)
+        return ResponseDelete.from_suds(suds_response=response, endpoint=endpoint)
 
     def _get_suds_service(self, endpoint: str) -> ServiceProxy:
         """Returns a SUDS service for the given endpoint. Reuses existing instances when possible."""

bfabric/src/bfabric/engine/engine_zeep.py

@@ -7,6 +7,7 @@
 from zeep.helpers import serialize_object
 
 from bfabric.errors import BfabricRequestError, get_response_errors
+from bfabric.results.response_delete import ResponseDelete
 from bfabric.results.result_container import ResultContainer
 from bfabric.results.response_format_dict import clean_result
 
@@ -104,15 +105,12 @@ def delete(self, endpoint: str, id: int | list[int], auth: BfabricAuth) -> Resul
         :param auth: the authentication handle of the user performing the request
         """
         if isinstance(id, list) and len(id) == 0:
-            print("Warning, attempted to delete an empty list, ignoring")
-            # TODO maybe use error here (and make sure it's consistent)
-            return ResultContainer([], total_pages_api=0)
+            return ResponseDelete.from_empty_request()
 
         query = {"login": auth.login, "password": auth.password.get_secret_value(), "id": id}
-
         client = self._get_client(endpoint)
         response = client.service.delete(query)
-        return self._convert_results(response=response, endpoint=endpoint)
+        return ResponseDelete.from_zeep(zeep_response=response, endpoint=endpoint)
 
     def _get_client(self, endpoint: str) -> zeep.Client:
         if endpoint not in self._cl:

bfabric/src/bfabric/entities/dataset.py

@@ -1,14 +1,14 @@
 from __future__ import annotations
 
-import tempfile
-from pathlib import Path
-from typing import Any, TYPE_CHECKING
+import io
+from typing import TYPE_CHECKING, Any
 
 from polars import DataFrame
 
 from bfabric.entities.core.entity import Entity
 
 if TYPE_CHECKING:
+    from pathlib import Path
     from bfabric import Bfabric
 
 
@@ -49,8 +49,14 @@ def write_csv(self, path: Path, separator: str = ",") -> None:
 
     def get_csv(self, separator: str = ",") -> str:
         """Returns the dataset as a csv string, using the specified column `separator`."""
-        with tempfile.NamedTemporaryFile() as tmp_file:
-            self.write_csv(Path(tmp_file.name), separator=separator)
-            tmp_file.flush()
-            tmp_file.seek(0)
-            return tmp_file.read().decode()
+        return self.to_polars().write_csv(separator=separator)
+
+    def write_parquet(self, path: Path) -> None:
+        """Writes the dataset to a parquet file at `path`."""
+        self.to_polars().write_parquet(path)
+
+    def get_parquet(self) -> bytes:
+        """Returns the dataset as a parquet bytes object."""
+        bytes_io = io.BytesIO()
+        self.to_polars().write_parquet(bytes_io)
+        return bytes_io.getvalue()

bfabric/src/bfabric/entities/workunit.py

@@ -10,6 +10,7 @@
 from bfabric.entities.core.has_container_mixin import HasContainerMixin
 from bfabric.entities.core.has_many import HasMany
 from bfabric.entities.core.has_one import HasOne
+from bfabric.utils.path_safe_name import path_safe_name
 
 if TYPE_CHECKING:
     from bfabric import Bfabric
@@ -52,8 +53,8 @@ def store_output_folder(self) -> Path:
         return Path(
             f"{self.application.storage['projectfolderprefix']}{self.container.id}",
             "bfabric",
-            self.application["technology"].replace(" ", "_"),
-            self.application["name"].replace(" ", "_"),
+            path_safe_name(self.application["technology"]),
+            path_safe_name(self.application["name"]),
             date.strftime("%Y/%Y-%m/%Y-%m-%d/"),
             f"workunit_{self.id}",
         )

bfabric/src/bfabric/errors.py

@@ -7,10 +7,20 @@ class BfabricRequestError(RuntimeError):
     """An error that is returned by the server in response to a full request."""
 
     def __init__(self, message: str) -> None:
+        # Call parent class constructor to properly initialize RuntimeError
+        super().__init__(message)
         self.message = message
 
     def __repr__(self) -> str:
-        return f"RequestError(message={repr(self.message)})"
+        return f"BfabricRequestError(message={repr(self.message)})"
+
+    def __str__(self) -> str:
+        return self.message
+
+    def __eq__(self, other: Any) -> bool:
+        if not isinstance(other, BfabricRequestError):
+            return False
+        return self.message == other.message
 
 
 class BfabricConfigError(RuntimeError):

bfabric/src/bfabric/experimental/multi_query.py

@@ -41,7 +41,7 @@ def read_multi(
         NOTE: It is assumed that there is only 1 response for each value.
         """
         # TODO add `check` parameter
-        response_tot = ResultContainer([], total_pages_api=0)
+        response_tot = ResultContainer([], total_pages_api=0, errors=[])
         obj_extended = deepcopy(obj)  # Make a copy of the query, not to make edits to the argument
 
         # Iterate over request chunks that fit into a single API page
@@ -79,7 +79,7 @@ def delete_multi(self, endpoint: str, id_list: list[int]) -> ResultContainer:
         """Deletes multiple objects from `endpoint` by their ids."""
         # TODO document and test error handling
         # TODO add `check` parameter
-        response_tot = ResultContainer([], total_pages_api=0)
+        response_tot = ResultContainer([], total_pages_api=0, errors=[])
 
         if not id_list:
             print("Warning, empty list provided for deletion, ignoring")

bfabric/src/bfabric/experimental/workunit_definition.py

@@ -6,6 +6,7 @@
 import yaml
 from bfabric.entities import Workunit
 from pydantic import BaseModel, model_validator
+from bfabric.utils.path_safe_name import PathSafeStr  # noqa: TC001
 
 if TYPE_CHECKING:
     from bfabric import Bfabric
@@ -61,11 +62,11 @@ class WorkunitRegistrationDefinition(BaseModel):
 
     application_id: int
     """The ID of the executing application."""
-    application_name: str
+    application_name: PathSafeStr
     """The name of the executing application."""
     workunit_id: int
     """The ID of the workunit."""
-    workunit_name: str
+    workunit_name: PathSafeStr
     """The name of the workunit."""
     container_id: int
     """The ID of the container."""

bfabric/src/bfabric/rest/token_data.py

@@ -1,7 +1,7 @@
 from __future__ import annotations
 
 from datetime import datetime
-from typing import TYPE_CHECKING
+from typing import TYPE_CHECKING, Any
 
 import requests
 from pydantic import BaseModel, Field, SecretStr, ConfigDict
@@ -13,9 +13,7 @@
 class TokenData(BaseModel):
     """Parsed token data from the B-Fabric token validation endpoint."""
 
-    model_config = ConfigDict(
-        populate_by_name=True, str_strip_whitespace=True, json_encoders={datetime: lambda v: v.isoformat()}
-    )
+    model_config = ConfigDict(populate_by_name=True, str_strip_whitespace=True)
 
     job_id: int = Field(alias="jobId")
     application_id: int = Field(alias="applicationId")
@@ -29,6 +27,14 @@ class TokenData(BaseModel):
     token_expires: datetime = Field(alias="expiryDateTime")
     environment: str
 
+    # Define a custom serializer method for model_dump
+    def model_dump(self, **kwargs: Any) -> dict[str, Any]:
+        data = super().model_dump(**kwargs)
+        # Convert datetime to ISO format
+        if "token_expires" in data and isinstance(data["token_expires"], datetime):
+            data["token_expires"] = data["token_expires"].isoformat()
+        return data
+
 
 def get_token_data(client_config: BfabricClientConfig, token: str) -> TokenData:
     """Returns the token data for the provided token.