Skip to content

Commit

Permalink
feat: run notebooks in data service (#375)
Browse files Browse the repository at this point in the history
Co-authored-by: Samuel Gaist <[email protected]>

squashme: resolve package version conflicts

feat: update and expand apispec for environments

chore: filter environments by owner type

squashme: address comments

feat!: expand environment specification

This is a breaking change in the API.

chore: add tests and minor fixes

chore: test the global environments migration

chore: fix tests

chore: minor improvements to db session handling

squashme: minor fix

squashme: fixups for conflict resolutuion after merge

squashme: fix failing tests

chore: address comments

feat: add command and args to environments

squashme: notebooks changes

This includes major edits to the notebooks code to work with the data
service.

chore: resolve changes from conflict resolution

chore: do not use the complicated notebooks gitlab header

The gitlab credentials header from the notebooks is really complicated.
We used it here just to get the access token expiry. I modified the
gateway to now pass in an extra header value to indicate the gitlab
token expiry.

squashme: handle per secret adoption in amalthea

squashme: fix parsing of PosixPath in orm

squashme: display the right status and state

squashme: address comments from review pt1

refactor: make APIUser a frozen dataclass

As there is no reason that these object shall be modified within
the services, it simplifies its handling.

squashme: address comments from review pt2

squashme: fixups from rebasing

squashme: use PurePosixPath for workdir and mount

squashme: add saved cloud storage model
  • Loading branch information
olevski committed Sep 23, 2024
1 parent 8878712 commit 72c23b8
Show file tree
Hide file tree
Showing 14 changed files with 68 additions and 68 deletions.
1 change: 1 addition & 0 deletions bases/renku_data_services/data_api/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,7 @@ def register_all_handlers(app: Sanic, config: Config) -> Sanic:
nb_config=config.nb_config,
project_repo=config.project_repo,
session_repo=config.session_repo,
storage_repo=config.storage_v2_repo,
rp_repo=config.rp_repo,
internal_gitlab_authenticator=config.gitlab_authenticator,
)
Expand Down
33 changes: 15 additions & 18 deletions components/renku_data_services/notebooks/api.spec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -439,27 +439,24 @@ components:
- registered
type: object
ErrorResponse:
type: object
properties:
error:
type: object
properties:
code:
type: integer
minimum: 0
exclusiveMinimum: true
example: 1404
detail:
type: string
example: "A more detailed optional message showing what the problem was"
message:
type: string
example: "Something went wrong - please try again later"
required:
- "code"
- "message"
"$ref": "#/components/schemas/ErrorResponseNested"
required:
- "error"
- error
type: object
ErrorResponseNested:
properties:
code:
type: integer
detail:
type: string
message:
type: string
required:
- code
- message
type: object
Generated:
properties:
enabled:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ async def main(server: "UserServer") -> list[dict[str, Any]]:
commit_sha = getattr(server, "commit_sha", None)

volume_mount = {
"mountPath": server.work_dir.absolute().as_posix(),
"mountPath": server.work_dir.as_posix(),
"name": "workspace",
}
if gl_project_path:
Expand Down Expand Up @@ -51,7 +51,7 @@ async def main(server: "UserServer") -> list[dict[str, Any]]:
"env": [
{
"name": "GIT_RPC_MOUNT_PATH",
"value": server.work_dir.absolute().as_posix(),
"value": server.work_dir.as_posix(),
},
{
"name": "GIT_RPC_PORT",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,11 @@ async def git_clone_container_v2(server: "UserServer") -> dict[str, Any] | None:
env = [
{
"name": f"{prefix}WORKSPACE_MOUNT_PATH",
"value": server.workspace_mount_path.absolute().as_posix(),
"value": server.workspace_mount_path.as_posix(),
},
{
"name": f"{prefix}MOUNT_PATH",
"value": server.work_dir.absolute().as_posix(),
"value": server.work_dir.as_posix(),
},
{
"name": f"{prefix}LFS_AUTO_FETCH",
Expand Down Expand Up @@ -134,7 +134,7 @@ async def git_clone_container_v2(server: "UserServer") -> dict[str, Any] | None:
},
"volumeMounts": [
{
"mountPath": server.workspace_mount_path.absolute().as_posix(),
"mountPath": server.workspace_mount_path.as_posix(),
"name": amalthea_session_work_volume,
},
*etc_cert_volume_mount,
Expand All @@ -161,11 +161,11 @@ async def git_clone_container(server: "UserServer") -> dict[str, Any] | None:
env = [
{
"name": f"{prefix}WORKSPACE_MOUNT_PATH",
"value": server.workspace_mount_path.absolute().as_posix(),
"value": server.workspace_mount_path.as_posix(),
},
{
"name": f"{prefix}MOUNT_PATH",
"value": server.work_dir.absolute().as_posix(),
"value": server.work_dir.as_posix(),
},
{
"name": f"{prefix}LFS_AUTO_FETCH",
Expand Down Expand Up @@ -260,7 +260,7 @@ async def git_clone_container(server: "UserServer") -> dict[str, Any] | None:
},
"volumeMounts": [
{
"mountPath": server.workspace_mount_path.absolute().as_posix(),
"mountPath": server.workspace_mount_path.as_posix(),
"name": "workspace",
},
*etc_cert_volume_mount,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ def env(server: "UserServer") -> list[dict[str, Any]]:
"path": "/statefulset/spec/template/spec/containers/0/env/-",
"value": {
"name": "NOTEBOOK_DIR",
"value": server.work_dir.absolute().as_posix(),
"value": server.work_dir.as_posix(),
},
},
{
Expand All @@ -53,7 +53,7 @@ def env(server: "UserServer") -> list[dict[str, Any]]:
# relative to $HOME.
"value": {
"name": "MOUNT_PATH",
"value": server.work_dir.absolute().as_posix(),
"value": server.work_dir.as_posix(),
},
},
{
Expand Down Expand Up @@ -223,7 +223,7 @@ def rstudio_env_variables(server: "UserServer") -> list[dict[str, Any]]:
"path": "/statefulset/spec/template/spec/containers/0/volumeMounts/-",
"value": {
"name": secret_name,
"mountPath": mount_location.absolute().as_posix(),
"mountPath": mount_location.as_posix(),
"subPath": mount_location.name,
"readOnly": True,
},
Expand Down
6 changes: 3 additions & 3 deletions components/renku_data_services/notebooks/api/classes/image.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
import re
from dataclasses import dataclass, field
from enum import Enum
from pathlib import Path
from pathlib import PurePosixPath
from typing import Any, Optional, Self, cast

import requests
Expand Down Expand Up @@ -101,7 +101,7 @@ def get_image_config(self, image: "Image") -> Optional[dict[str, Any]]:
return None
return cast(dict[str, Any], res.json())

def image_workdir(self, image: "Image") -> Optional[Path]:
def image_workdir(self, image: "Image") -> Optional[PurePosixPath]:
"""Query the docker API to get the workdir of an image."""
config = self.get_image_config(image)
if config is None:
Expand All @@ -112,7 +112,7 @@ def image_workdir(self, image: "Image") -> Optional[Path]:
workdir = nested_config.get("WorkingDir", "/")
if workdir == "":
workdir = "/"
return Path(workdir)
return PurePosixPath(workdir)

def with_oauth2_token(self, oauth2_token: str) -> "ImageRepoDockerAPI":
"""Return a docker API instance with the token as authentication."""
Expand Down
20 changes: 10 additions & 10 deletions components/renku_data_services/notebooks/api/classes/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
from abc import ABC
from collections.abc import Sequence
from itertools import chain
from pathlib import Path
from pathlib import PurePosixPath
from typing import Any
from urllib.parse import urljoin, urlparse

Expand Down Expand Up @@ -44,8 +44,8 @@ def __init__(
user_secrets: K8sUserSecrets | None,
cloudstorage: Sequence[ICloudStorageRequest],
k8s_client: K8sClient,
workspace_mount_path: Path,
work_dir: Path,
workspace_mount_path: PurePosixPath,
work_dir: PurePosixPath,
config: _NotebooksConfig,
internal_gitlab_user: APIUser,
using_default_image: bool = False,
Expand Down Expand Up @@ -204,7 +204,7 @@ async def _get_session_manifest(self) -> dict[str, Any]:
"pvc": {
"enabled": True,
"storageClassName": self.config.sessions.storage.pvs_storage_class,
"mountPath": self.workspace_mount_path.absolute().as_posix(),
"mountPath": self.workspace_mount_path.as_posix(),
},
}
else:
Expand All @@ -213,7 +213,7 @@ async def _get_session_manifest(self) -> dict[str, Any]:
"size": storage_size,
"pvc": {
"enabled": False,
"mountPath": self.workspace_mount_path.absolute().as_posix(),
"mountPath": self.workspace_mount_path.as_posix(),
},
}
# Authentication
Expand Down Expand Up @@ -256,7 +256,7 @@ async def _get_session_manifest(self) -> dict[str, Any]:
"jupyterServer": {
"defaultUrl": self.server_options.default_url,
"image": self.image,
"rootDir": self.work_dir.absolute().as_posix(),
"rootDir": self.work_dir.as_posix(),
"resources": self.server_options.to_k8s_resources(
enforce_cpu_limits=self.config.sessions.enforce_cpu_limits
),
Expand Down Expand Up @@ -375,8 +375,8 @@ def __init__(
user_secrets: K8sUserSecrets | None,
cloudstorage: Sequence[ICloudStorageRequest],
k8s_client: K8sClient,
workspace_mount_path: Path,
work_dir: Path,
workspace_mount_path: PurePosixPath,
work_dir: PurePosixPath,
config: _NotebooksConfig,
gitlab_client: NotebooksGitlabClient,
internal_gitlab_user: APIUser,
Expand Down Expand Up @@ -502,8 +502,8 @@ def __init__(
user_secrets: K8sUserSecrets | None,
cloudstorage: Sequence[ICloudStorageRequest],
k8s_client: K8sClient,
workspace_mount_path: Path,
work_dir: Path,
workspace_mount_path: PurePosixPath,
work_dir: PurePosixPath,
repositories: list[Repository],
config: _NotebooksConfig,
internal_gitlab_user: APIUser,
Expand Down
18 changes: 8 additions & 10 deletions components/renku_data_services/notebooks/apispec.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,16 +34,10 @@ class DefaultCullingThresholds(BaseAPISpec):
registered: CullingThreshold


class Error(BaseAPISpec):
code: int = Field(..., example=1404, gt=0)
detail: Optional[str] = Field(
None, example="A more detailed optional message showing what the problem was"
)
message: str = Field(..., example="Something went wrong - please try again later")


class ErrorResponse(BaseAPISpec):
error: Error
class ErrorResponseNested(BaseAPISpec):
code: int
detail: Optional[str] = None
message: str


class Generated(BaseAPISpec):
Expand Down Expand Up @@ -299,6 +293,10 @@ class SessionsImagesGetParametersQuery(BaseAPISpec):
image_url: str


class ErrorResponse(BaseAPISpec):
error: ErrorResponseNested


class LaunchNotebookRequest(BaseAPISpec):
project_id: str
launcher_id: str
Expand Down
2 changes: 0 additions & 2 deletions components/renku_data_services/storage/blueprints.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@ async def _get(
validator: RCloneValidator,
query: apispec.StorageParams,
) -> JSONResponse:
storage: list[models.CloudStorage]
storage = await self.storage_repo.get_storage(user=user, project_id=query.project_id)

return json([dump_storage_with_sensitive_fields(s, validator) for s in storage])
Expand Down Expand Up @@ -202,7 +201,6 @@ async def _get(
validator: RCloneValidator,
query: apispec.StorageV2Params,
) -> JSONResponse:
storage: list[models.CloudStorage]
storage = await self.storage_v2_repo.get_storage(
user=user, include_secrets=True, project_id=query.project_id
)
Expand Down
12 changes: 6 additions & 6 deletions components/renku_data_services/storage/db.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ async def get_storage(
name: str | None = None,
include_secrets: bool = False,
filter_by_access_level: bool = True,
) -> list[models.CloudStorage]:
) -> list[models.SavedCloudStorage]:
"""Get a storage from the database."""
async with self.session_maker() as session:
if not project_id and not name and not id:
Expand Down Expand Up @@ -91,7 +91,7 @@ async def get_storage(

return [s.dump() for s in storage_orms if s.project_id in accessible_projects]

async def get_storage_by_id(self, storage_id: ULID, user: base_models.APIUser) -> models.CloudStorage:
async def get_storage_by_id(self, storage_id: ULID, user: base_models.APIUser) -> models.SavedCloudStorage:
"""Get a single storage by id."""
storages = await self.get_storage(user, id=str(storage_id), include_secrets=True, filter_by_access_level=False)

Expand All @@ -102,9 +102,7 @@ async def get_storage_by_id(self, storage_id: ULID, user: base_models.APIUser) -

return storages[0]

async def insert_storage(
self, storage: models.UnsavedCloudStorage, user: base_models.APIUser
) -> models.CloudStorage:
async def insert_storage(self, storage: models.CloudStorage, user: base_models.APIUser) -> models.SavedCloudStorage:
"""Insert a new cloud storage entry."""
if not await self.filter_projects_by_access_level(user, [storage.project_id], authz_models.Role.OWNER):
raise errors.ForbiddenError(message="User does not have access to this project")
Expand All @@ -118,7 +116,9 @@ async def insert_storage(
session.add(orm)
return orm.dump()

async def update_storage(self, storage_id: ULID, user: base_models.APIUser, **kwargs: dict) -> models.CloudStorage:
async def update_storage(
self, storage_id: ULID, user: base_models.APIUser, **kwargs: dict
) -> models.SavedCloudStorage:
"""Update a cloud storage entry."""
async with self.session_maker() as session, session.begin():
res = await session.execute(
Expand Down
6 changes: 6 additions & 0 deletions components/renku_data_services/storage/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -253,3 +253,9 @@ class CloudStorageSecretUpsert(BaseModel):

name: str = Field()
value: str = Field()


class SavedCloudStorage(CloudStorage):
"""A cloud storage that has been saved in the DB."""

storage_id: ULID
4 changes: 2 additions & 2 deletions components/renku_data_services/storage/orm.py
Original file line number Diff line number Diff line change
Expand Up @@ -81,9 +81,9 @@ def load(cls, storage: models.UnsavedCloudStorage) -> "CloudStorageORM":
readonly=storage.readonly,
)

def dump(self) -> models.CloudStorage:
def dump(self) -> models.SavedCloudStorage:
"""Create a cloud storage model from the ORM object."""
return models.CloudStorage(
return models.SavedCloudStorage(
project_id=self.project_id,
name=self.name,
storage_type=self.storage_type,
Expand Down
6 changes: 3 additions & 3 deletions poetry.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions projects/renku_data_service/poetry.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 72c23b8

Please sign in to comment.