Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 2 additions & 6 deletions gateway/api/ray.py
Original file line number Diff line number Diff line change
Expand Up @@ -223,24 +223,20 @@ def _create_cluster_data(job: Job, cluster_name: str):

# configure provider configuration if needed
node_image = settings.RAY_NODE_IMAGE
provider_name = None
if job.program.provider is not None:
node_image = job.program.image
provider_name = job.program.provider.name

user_file_storage = FileStorage(
username=user.username,
working_dir=WorkingDir.USER_STORAGE,
function_title=job.program.title,
provider_name=provider_name,
function=job.program,
)
provider_file_storage = user_file_storage
if job.program.provider is not None:
provider_file_storage = FileStorage(
username=user.username,
working_dir=WorkingDir.PROVIDER_STORAGE,
function_title=job.program.title,
provider_name=provider_name,
function=job.program,
)

cluster = get_template("rayclustertemplate.yaml")
Expand Down
6 changes: 1 addition & 5 deletions gateway/api/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -271,8 +271,6 @@ def create(self, validated_data):
logger.info("Creating Job with RunExistingJobSerializer")
status = Job.QUEUED
program = validated_data.get("program")
function_title = validated_data.get("function_title")
provider_name = validated_data.get("provider_name", None)
arguments = validated_data.get("arguments", "{}")
author = validated_data.get("author")
config = validated_data.get("config", None)
Expand Down Expand Up @@ -301,9 +299,7 @@ def create(self, validated_data):
)
)

arguments_storage = ArgumentsStorage(
author.username, function_title, provider_name
)
arguments_storage = ArgumentsStorage(author.username, program)
arguments_storage.save(job.id, arguments)

try:
Expand Down
16 changes: 13 additions & 3 deletions gateway/api/services/arguments_storage.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
from typing import Optional
from django.conf import settings

from api.models import Program

logger = logging.getLogger("gateway")


Expand All @@ -15,9 +17,17 @@ class ArgumentsStorage:
ARGUMENTS_FILE_EXTENSION = ".json"
ENCODING = "utf-8"

def __init__(
self, username: str, function_title: str, provider_name: Optional[str]
):
def __init__(self, username: str, function: Program):
"""
Initialize ArgumentsStorage with a function instance.

Args:
username: Program model instance containing title, provider, and author
function: Program model instance containing title, provider, and author
"""
function_title = function.title
provider_name = function.provider.name if function.provider else None

# We need to use the same path as the FileStorage here
# because it is attached the volume in the docker image
if provider_name is None:
Expand Down
20 changes: 12 additions & 8 deletions gateway/api/services/file_storage.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,25 +35,29 @@ class WorkingDir(Enum):
class FileStorage:
"""
The main objective of this class is to manage the access of the users to their storage.

Attributes:
username (str): storage user's username
working_dir (WorkingDir(Enum)): working directory
function_title (str): title of the function in case is needed to build the path
provider_name (str | None): name of the provider in caseis needed to build the path
"""

def __init__(
self,
username: str,
working_dir: WorkingDir,
function_title: str,
provider_name: Optional[str],
function,
) -> None:
"""
Initialize FileStorage with a function instance.

Args:
username: User's username
working_dir: Working directory type (USER_STORAGE or PROVIDER_STORAGE)
function: Program model instance containing title and provider
"""
self.sub_path = None
self.absolute_path = None
self.username = username

function_title = function.title
provider_name = function.provider.name if function.provider else None

if working_dir is WorkingDir.USER_STORAGE:
self.sub_path = self.__get_user_sub_path(function_title, provider_name)
elif working_dir is WorkingDir.PROVIDER_STORAGE:
Expand Down
10 changes: 8 additions & 2 deletions gateway/api/services/result_storage.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,14 @@ class ResultStorage:
RESULT_FILE_EXTENSION = ".json"
ENCODING = "utf-8"

def __init__(self, username: str):
"""Initialize the storage path for a given user."""
def __init__(self, function):
"""
Initialize the storage path for a given function.

Args:
function: Program model instance containing author
"""
username = function.author.username
self.user_results_directory = os.path.join(
settings.MEDIA_ROOT, username, "results"
)
Comment on lines +18 to 28
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this changes is putting the result data in the wrong folder. The folder should be the user who runs the job. That means, this change is not needed at all.

Expand Down
3 changes: 1 addition & 2 deletions gateway/api/use_cases/files/delete.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,7 @@ def execute(
file_storage = FileStorage(
username=user.username,
working_dir=self.working_dir,
function_title=function_title,
provider_name=provider_name,
function=function,
)
result = file_storage.remove_file(file_name=file_name)

Expand Down
3 changes: 1 addition & 2 deletions gateway/api/use_cases/files/download.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,7 @@ def execute(
file_storage = FileStorage(
username=user.username,
working_dir=self.working_dir,
function_title=function_title,
provider_name=provider_name,
function=function,
)
result = file_storage.get_file(file_name=requested_file_name)

Expand Down
3 changes: 1 addition & 2 deletions gateway/api/use_cases/files/list.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,7 @@ def execute(self, user: AbstractUser, provider_name: str, function_title: str):
file_storage = FileStorage(
username=user.username,
working_dir=self.working_dir,
function_title=function_title,
provider_name=provider_name,
function=function,
)

return file_storage.get_files()
3 changes: 1 addition & 2 deletions gateway/api/use_cases/files/provider_delete.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,7 @@ def execute(
file_storage = FileStorage(
username=user.username,
working_dir=self.working_dir,
function_title=function_title,
provider_name=provider_name,
function=function,
)
result = file_storage.remove_file(file_name=file_name)

Expand Down
3 changes: 1 addition & 2 deletions gateway/api/use_cases/files/provider_download.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,7 @@ def execute(
file_storage = FileStorage(
username=user.username,
working_dir=self.working_dir,
function_title=function_title,
provider_name=provider_name,
function=function,
)
result = file_storage.get_file(file_name=requested_file_name)

Expand Down
3 changes: 1 addition & 2 deletions gateway/api/use_cases/files/provider_list.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,7 @@ def execute(self, user: AbstractUser, provider_name: str, function_title: str):
file_storage = FileStorage(
username=user.username,
working_dir=self.working_dir,
function_title=function_title,
provider_name=provider_name,
function=function,
)

return file_storage.get_files()
3 changes: 1 addition & 2 deletions gateway/api/use_cases/files/provider_upload.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,7 @@ def execute(
file_storage = FileStorage(
username=user.username,
working_dir=self.working_dir,
function_title=function_title,
provider_name=provider_name,
function=function,
)
result = file_storage.upload_file(file=uploaded_file)

Expand Down
3 changes: 1 addition & 2 deletions gateway/api/use_cases/files/upload.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,7 @@ def execute(
file_storage = FileStorage(
username=user.username,
working_dir=self.working_dir,
function_title=function_title,
provider_name=provider_name,
function=function,
)
result = file_storage.upload_file(file=uploaded_file)

Expand Down
2 changes: 1 addition & 1 deletion gateway/api/use_cases/jobs/retrieve.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ def execute(self, job_id: UUID, user: AbstractUser, with_result: bool) -> Job:
can_read_result = JobAccessPolicies.can_read_result(user, job)

if with_result and can_read_result:
result_store = ResultStorage(user.username)
result_store = ResultStorage(job.program)
result = result_store.get(str(job.id))
if result is not None:
job.result = result
Expand Down
2 changes: 1 addition & 1 deletion gateway/api/use_cases/jobs/save_result.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ def execute(self, job_id: UUID, user: AbstractUser, result: str) -> Job:
if not can_save_result:
raise NotFoundError(f"Job [{job_id}] not found")

result_storage = ResultStorage(user.username)
result_storage = ResultStorage(job.program)
result_storage.save(job.id, result)
job.result = result

Expand Down
8 changes: 4 additions & 4 deletions gateway/tests/api/test_v1_program.py
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,8 @@ def test_run(self):
self.assertEqual(job.config.workers, None)
self.assertEqual(job.config.auto_scaling, True)

arguments_storage = ArgumentsStorage(user.username, "Program", None)
program = Program.objects.get(title="Program", author=user)
arguments_storage = ArgumentsStorage(user.username, program)
stored_arguments = arguments_storage.get(job.id)

self.assertEqual(stored_arguments, arguments)
Expand Down Expand Up @@ -195,9 +196,8 @@ def test_provider_run(self):
self.assertEqual(job.config.workers, None)
self.assertEqual(job.config.auto_scaling, True)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should assert if the folder name where the result/argument/logs/whatever is created is right, not only the content.

arguments_storage = ArgumentsStorage(
user.username, "Docker-Image-Program", "default"
)
program = Program.objects.get(title="Docker-Image-Program", author=user)
arguments_storage = ArgumentsStorage(user.username, program)
stored_arguments = arguments_storage.get(job.id)

self.assertEqual(stored_arguments, arguments)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
upgrade:
- |
Refactored storage service initialization to accept ``Program`` model instances
instead of individual parameters. The ``FileStorage``, ``ArgumentsStorage``, and
``ResultStorage`` classes now accept a ``function`` parameter (Program model instance)
rather than separate ``username``, ``function_title``, and ``provider_name`` parameters.
This change improves code cohesion by reducing the number of parameters passed between
components and ensures consistent access to program metadata across storage services.