Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/aqua ms changes #1019

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
+672 −10
Open

Feature/aqua ms changes #1019

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
da78f8c
Added methods for aqua-model store apis change
vikasray0208 Dec 2, 2024
e74334d
update modela and mvs list api
vikasray0208 Dec 5, 2024
d25c442
Added methods for aqua-model store apis change
vikasray0208 Dec 2, 2024
b1a208d
update modela and mvs list api
vikasray0208 Dec 5, 2024
81db37c
Merge remote-tracking branch 'origin/main' into feature/aqua_ms_changes
vikasray0208 Dec 13, 2024
01bb578
Merge remote-tracking branch 'origin/feature/aqua_ms_changes' into fe…
vikasray0208 Dec 13, 2024
1e98882
sync with feature branch
vikasray0208 Dec 13, 2024
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
260 changes: 256 additions & 4 deletions ads/model/datascience_model.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,10 @@
import os
import shutil
import tempfile
import uuid
from copy import deepcopy
from typing import Dict, List, Optional, Tuple, Union
from zipfile import ZipFile

import pandas
import yaml
Expand Down Expand Up @@ -1571,7 +1573,7 @@ def delete(

@classmethod
def list(
cls, compartment_id: str = None, project_id: str = None, **kwargs
cls, compartment_id: str = None, project_id: str = None, category: str = "USER", **kwargs
Copy link
Member

Choose a reason for hiding this comment

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

Let's move category to constants.

class ModelCategory (str, metaclass=ExtendedEnumMeta):
    USER = "USER"
    SERVICE = "SERVICE"

This variable is used across many modules.

) -> List["DataScienceModel"]:
"""Lists datascience models in a given compartment.

Expand All @@ -1581,6 +1583,8 @@ def list(
The compartment OCID.
project_id: (str, optional). Defaults to `None`.
The project OCID.
category: (str, optional). Defaults to `USER`.
The category of Model. Allowed values are: "USER", "SERVICE"
kwargs
Additional keyword arguments for filtering models.

Expand All @@ -1592,13 +1596,13 @@ def list(
return [
cls()._update_from_oci_dsc_model(model)
for model in OCIDataScienceModel.list_resource(
compartment_id, project_id=project_id, **kwargs
compartment_id, project_id=project_id, category=category, **kwargs
)
]

@classmethod
def list_df(
cls, compartment_id: str = None, project_id: str = None, **kwargs
cls, compartment_id: str = None, project_id: str = None, category: str = "USER", **kwargs
Copy link
Member

Choose a reason for hiding this comment

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

Could you install a pre-commit hook and validate the changed files with it. Internally we use ruff formatter.

) -> "pandas.DataFrame":
"""Lists datascience models in a given compartment.

Expand All @@ -1608,6 +1612,8 @@ def list_df(
The compartment OCID.
project_id: (str, optional). Defaults to `None`.
The project OCID.
category: (str, optional). Defaults to `None`.
The category of Model.
kwargs
Additional keyword arguments for filtering models.

Expand All @@ -1618,7 +1624,7 @@ def list_df(
"""
records = []
for model in OCIDataScienceModel.list_resource(
compartment_id, project_id=project_id, **kwargs
compartment_id, project_id=project_id, category=category, **kwargs
):
records.append(
{
Expand Down Expand Up @@ -2193,3 +2199,249 @@ def find_model_idx():
else:
# model found case
self.model_file_description["models"].pop(modelSearchIdx)

def create_custom_metadata_artifact(self, model_ocid: str, metadata_key_name: str, artifact_path: str) -> dict:
"""Creates model custom metadata artifact for specified model.
Copy link
Member

Choose a reason for hiding this comment

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

Please add more description for this model. For example when this method needs to be used. Also check the parameters section, looks like there is some formatting issue.
Also it would be better if instead of dict we return some dataclass or pydentic class.

from ads.common.serializer import DataClassSerializable

@dataclass
class MyClass():

    attr1: Optional[str] = None
    attr2: Optional[str] = None

OR

from pydantic import BaseModel

class MyClass(BaseModel):

    attr1: Optional[str] = None
    attr2: Optional[str] = None


Parameters
----------
model_ocid: str
Copy link
Member

Choose a reason for hiding this comment

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

we don't need to accept model_ocid as input for these functions, can we use the self.id property instead?

The `OCID`__ of the model.
__ https://docs.cloud.oracle.com/iaas/Content/General/Concepts/identifiers.htm

metadata_key_name: str
The name of the model metadatum in the metadata.

artifact_path: str
Copy link
Member

Choose a reason for hiding this comment

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

I suppose we will support only local paths, not OS bucket paths? If so, that needs to be mentioned in the pydocs.

The model custom metadata artifact path to be upload.
Returns
-------
Dict
The model custom metadata artifact creation info.
Example:
{
'Date': 'Mon, 02 Dec 2024 06:38:24 GMT',
'opc-request-id': 'E4F7',
'ETag': '77156317-8bb9-4c4a-882b-0d85f8140d93',
'X-Content-Type-Options': 'nosniff',
'Content-Length': '4029958',
'Vary': 'Origin',
'Strict-Transport-Security': 'max-age=31536000; includeSubDomains',
'status': 204
}

"""
return self.dsc_model.create_custom_metadata_artifact(model_ocid=model_ocid, metadata_key_name=metadata_key_name,
artifact_path=artifact_path)


def create_defined_metadata_artifact(self, model_ocid: str, metadata_key_name: str, artifact_path: str) -> dict:
Copy link
Member

Choose a reason for hiding this comment

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

Why can't we have a one method and pass artifact type as a param?

def create_defined_metadata_artifact(..., type: str):
       return self.dsc_model.create_defined_metadata_artifact(...)

"""Creates model defined metadata artifact for specified model.

Parameters
----------
model_ocid: str
The `OCID`__ of the model.
__ https://docs.cloud.oracle.com/iaas/Content/General/Concepts/identifiers.htm

metadata_key_name: str
The name of the model metadatum in the metadata.

artifact_path: str
The model custom metadata artifact path to be upload.
Returns
-------
The model defined metadata artifact creation info.
Example:
{
'Date': 'Mon, 02 Dec 2024 06:38:24 GMT',
'opc-request-id': 'E4F7',
'ETag': '77156317-8bb9-4c4a-882b-0d85f8140d93',
'X-Content-Type-Options': 'nosniff',
'Content-Length': '4029958',
'Vary': 'Origin',
'Strict-Transport-Security': 'max-age=31536000; includeSubDomains',
'status': 204
}

"""
return self.dsc_model.create_defined_metadata_artifact(model_ocid=model_ocid, metadata_key_name=metadata_key_name,
artifact_path=artifact_path)


def update_custom_metadata_artifact(self, model_ocid: str, metadata_key_name: str, artifact_path: str) -> dict:
Copy link
Member

Choose a reason for hiding this comment

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

Same here, why can't we have just one common method for updating artifacts?

"""Update model custom metadata artifact for specified model.

Parameters
----------
model_ocid: str
The `OCID`__ of the model.
__ https://docs.cloud.oracle.com/iaas/Content/General/Concepts/identifiers.htm

metadata_key_name: str
The name of the model metadatum in the metadata.

artifact_path: str
The model custom metadata artifact path to be upload.
Returns
-------
Dict
The model custom metadata artifact update info.
Example:
{
'Date': 'Mon, 02 Dec 2024 06:38:24 GMT',
'opc-request-id': 'E4F7',
'ETag': '77156317-8bb9-4c4a-882b-0d85f8140d93',
'X-Content-Type-Options': 'nosniff',
'Content-Length': '4029958',
'Vary': 'Origin',
'Strict-Transport-Security': 'max-age=31536000; includeSubDomains',
'status': 204
}

"""
return self.dsc_model.update_custom_metadata_artifact(model_ocid=model_ocid, metadata_key_name=metadata_key_name,
artifact_path=artifact_path)


def update_defined_metadata_artifact(self, model_ocid: str, metadata_key_name: str, artifact_path: str) -> dict:
"""Update model defined metadata artifact for specified model.

Parameters
----------
model_ocid: str
The `OCID`__ of the model.
__ https://docs.cloud.oracle.com/iaas/Content/General/Concepts/identifiers.htm

metadata_key_name: str
The name of the model metadatum in the metadata.

artifact_path: str
The model defined metadata artifact path to be upload.
Returns
-------
Dict
The model defined metadata artifact update info.
Example:
{
'Date': 'Mon, 02 Dec 2024 06:38:24 GMT',
'opc-request-id': 'E4F7',
'ETag': '77156317-8bb9-4c4a-882b-0d85f8140d93',
'X-Content-Type-Options': 'nosniff',
'Content-Length': '4029958',
'Vary': 'Origin',
'Strict-Transport-Security': 'max-age=31536000; includeSubDomains',
'status': 204
}

"""
return self.dsc_model.update_defined_metadata_artifact(model_ocid=model_ocid, metadata_key_name=metadata_key_name,
artifact_path=artifact_path)

def get_custom_metadata_artifact(self, model_ocid: str, metadata_key_name: str, target_dir: str) -> None:
"""Downloads model custom metadata artifact content for specified model metadata key.

Parameters
----------
model_ocid: str
The `OCID`__ of the model.
__ https://docs.cloud.oracle.com/iaas/Content/General/Concepts/identifiers.htm

metadata_key_name: str
The name of the model metadatum in the metadata.
Returns
-------
BytesIO
custom metadata artifact content

"""
file_content = self.dsc_model.get_custom_metadata_artifact(model_ocid=model_ocid,
metadata_key_name=metadata_key_name)
artifact_file_path = os.path.join(
target_dir, f"{metadata_key_name}"
)
with open(artifact_file_path, "wb") as _file:
Copy link
Member

Choose a reason for hiding this comment

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

What if the file with the given name already exists? We should allow user to override existing files and fail if they didn't set override param to True, by default it should be false.

_file.write(file_content)
print(f"Artifact downloaded to location - {artifact_file_path}")
Copy link
Member

Choose a reason for hiding this comment

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

use logger instead of print throughout the PR


def get_defined_metadata_artifact(self, model_ocid: str, metadata_key_name: str, target_dir: str) -> None:
"""Downloads model defined metadata artifact content for specified model metadata key.

Parameters
----------
model_ocid: str
The `OCID`__ of the model.
__ https://docs.cloud.oracle.com/iaas/Content/General/Concepts/identifiers.htm

metadata_key_name: str
The name of the model metadatum in the metadata.
Returns
-------
BytesIO
Defined metadata artifact content

"""
file_content = self.dsc_model.get_defined_metadata_artifact(model_ocid=model_ocid,
metadata_key_name=metadata_key_name)
artifact_file_path = os.path.join(
target_dir, f"{metadata_key_name}"
)
with open(artifact_file_path, "wb") as _file:
_file.write(file_content)
print(f"Artifact downloaded to location - {artifact_file_path}")

def delete_custom_metadata_artifact(self, model_ocid: str, metadata_key_name: str) -> dict:
"""Deletes model custom metadata artifact for specified model metadata key.

Parameters
----------
model_ocid: str
The `OCID`__ of the model.
__ https://docs.cloud.oracle.com/iaas/Content/General/Concepts/identifiers.htm

metadata_key_name: str
The name of the model metadatum in the metadata.
Returns
-------
Dict
The model custom metadata artifact delete call info.
Example:
{
'Date': 'Mon, 02 Dec 2024 06:38:24 GMT',
'opc-request-id': 'E4F7',
'X-Content-Type-Options': 'nosniff',
'Vary': 'Origin',
'Strict-Transport-Security': 'max-age=31536000; includeSubDomains',
'status': 204
}

"""
return self.dsc_model.delete_custom_metadata_artifact(model_ocid=model_ocid, metadata_key_name=metadata_key_name)

def delete_defined_metadata_artifact(self, model_ocid: str, metadata_key_name: str) -> dict:
"""Deletes model defined metadata artifact for specified model metadata key.

Parameters
----------
model_ocid: str
The `OCID`__ of the model.
__ https://docs.cloud.oracle.com/iaas/Content/General/Concepts/identifiers.htm

metadata_key_name: str
The name of the model metadatum in the metadata.
Returns
-------
Dict
The model defined metadata artifact delete call info.
Example:
{
'Date': 'Mon, 02 Dec 2024 06:38:24 GMT',
'opc-request-id': 'E4F7',
'X-Content-Type-Options': 'nosniff',
'Vary': 'Origin',
'Strict-Transport-Security': 'max-age=31536000; includeSubDomains',
'status': 204
}

"""
return self.dsc_model.delete_defined_metadata_artifact(model_ocid=model_ocid, metadata_key_name=metadata_key_name)
6 changes: 4 additions & 2 deletions ads/model/model_version_set.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -454,14 +454,16 @@ def kind(self) -> str:
return "modelVersionSet"

@classmethod
def list(cls, compartment_id: str = None, **kwargs) -> List["ModelVersionSet"]:
def list(cls, compartment_id: str = None, category: str = "USER", **kwargs) -> List["ModelVersionSet"]:
Copy link
Member

Choose a reason for hiding this comment

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

Let's move category to constants.

class ModelCategory (str, metaclass=ExtendedEnumMeta):
    USER = "USER"
    SERVICE = "SERVICE"

This variable is used across many modules.

Copy link
Member

Choose a reason for hiding this comment

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

Create will continue working without changes, right?

"""
List model version sets in a given compartment.

Parameters
----------
compartment_id: str
The OCID of compartment.
category: (str, optional). Defaults to `USER`.
The category of Model. Allowed values are: "USER", "SERVICE"
kwargs
Additional keyword arguments for filtering model version sets.

Expand All @@ -473,7 +475,7 @@ def list(cls, compartment_id: str = None, **kwargs) -> List["ModelVersionSet"]:
return [
cls.from_dsc_model_version_set(model_version_set)
for model_version_set in DataScienceModelVersionSet.list_resource(
compartment_id, **kwargs
compartment_id, category=category, **kwargs
)
]

Expand Down
Loading
Loading