Skip to content

Commit

Permalink
[Project] Change default new_project to create instead of store (#2279
Browse files Browse the repository at this point in the history
)
  • Loading branch information
alonmr committed Aug 24, 2022
1 parent ffa4993 commit 23f0e5f
Show file tree
Hide file tree
Showing 5 changed files with 279 additions and 32 deletions.
10 changes: 8 additions & 2 deletions mlrun/api/crud/projects.py
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,12 @@ def _verify_project_has_no_external_resources(self, project: str):
# an MLRun resource (such as model-endpoints) was already verified in previous checks. Therefore, any internal
# secret existing here is something that the user needs to be notified about, as MLRun didn't generate it.
# Therefore, this check should remain at the end of the verification flow.
if mlrun.api.utils.singletons.k8s.get_k8s().get_project_secret_keys(project):
if (
mlrun.mlconf.is_api_running_on_k8s()
and mlrun.api.utils.singletons.k8s.get_k8s().get_project_secret_keys(
project
)
):
raise mlrun.errors.MLRunPreconditionFailedError(
f"Project {project} can not be deleted since related resources found: project secrets"
)
Expand Down Expand Up @@ -145,7 +150,8 @@ def delete_project_resources(
mlrun.api.crud.ModelEndpoints().delete_model_endpoints_resources(name)

# delete project secrets - passing None will delete all secrets
mlrun.api.utils.singletons.k8s.get_k8s().delete_project_secrets(name, None)
if mlrun.mlconf.is_api_running_on_k8s():
mlrun.api.utils.singletons.k8s.get_k8s().delete_project_secrets(name, None)

def get_project(
self, session: sqlalchemy.orm.Session, name: str
Expand Down
48 changes: 43 additions & 5 deletions mlrun/projects/project.py
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ def new_project(
description: str = None,
subpath: str = None,
save: bool = True,
overwrite: bool = False,
) -> "MlrunProject":
"""Create a new MLRun project, optionally load it from a yaml/zip/git template
Expand Down Expand Up @@ -133,6 +134,8 @@ def new_project(
:param description: text describing the project
:param subpath: project subpath (relative to the context dir)
:param save: whether to save the created project in the DB
:param overwrite: overwrite project using 'cascade' deletion strategy (deletes project resources)
if project with name exists
:returns: project object
"""
Expand Down Expand Up @@ -169,7 +172,26 @@ def new_project(
mlrun.mlconf.default_project = project.metadata.name
pipeline_context.set(project)
if save and mlrun.mlconf.dbpath:
project.save()
if overwrite:
logger.info(f"Deleting project {name} from MLRun DB due to overwrite")
_delete_project_from_db(
name, secrets, mlrun.api.schemas.DeletionStrategy.cascade
)

try:
project.save(store=False)
except mlrun.errors.MLRunConflictError as exc:
raise mlrun.errors.MLRunConflictError(
f"Project with name {name} already exists. "
"Use overwrite=True to overwrite the existing project."
) from exc
logger.info(
f"Created and saved project {name}",
from_template=from_template,
overwrite=overwrite,
context=context,
save=save,
)
return project


Expand Down Expand Up @@ -400,6 +422,11 @@ def _load_project_from_db(url, secrets, user_project=False):
return db.get_project(project_name)


def _delete_project_from_db(project_name, secrets, deletion_strategy):
db = mlrun.db.get_run_db(secrets=secrets)
return db.delete_project(project_name, deletion_strategy=deletion_strategy)


def _load_project_file(url, name="", secrets=None):
try:
obj = get_object(url, secrets)
Expand Down Expand Up @@ -2031,14 +2058,25 @@ def clear_context(self):
):
shutil.rmtree(self.spec.context)

def save(self, filepath=None):
def save(self, filepath=None, store=True):
"""export project to yaml file and save project in database
:store: if True, allow updating in case project already exists
"""
self.export(filepath)
self.save_to_db()
self.save_to_db(store)
return self

def save_to_db(self):
def save_to_db(self, store=True):
"""save project to database
:store: if True, allow updating in case project already exists
"""
db = mlrun.db.get_run_db(secrets=self._secrets)
db.store_project(self.metadata.name, self.to_dict())
if store:
return db.store_project(self.metadata.name, self.to_dict())

return db.create_project(self.to_dict())

def export(self, filepath=None, include_files: str = None):
"""save the project object into a yaml file or zip archive (default to project.yaml)
Expand Down
2 changes: 2 additions & 0 deletions tests/api/api/test_projects.py
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,7 @@ def test_delete_project_with_resources(
# need to set this to False, otherwise impl will try to delete k8s resources, and will need many more
# mocks to overcome this.
k8s_secrets_mock.set_is_running_in_k8s_cluster(False)
mlrun.mlconf.namespace = "test-namespace"
project_to_keep = "project-to-keep"
project_to_remove = "project-to-remove"
_create_resources_of_all_kinds(db, k8s_secrets_mock, project_to_keep)
Expand Down Expand Up @@ -631,6 +632,7 @@ def test_delete_project_deletion_strategy_check_external_resource(
project_member_mode: str,
k8s_secrets_mock: tests.api.conftest.K8sSecretsMock,
) -> None:
mlrun.mlconf.namespace = "test-namespace"
project = mlrun.api.schemas.Project(
metadata=mlrun.api.schemas.ProjectMetadata(name="project-name"),
spec=mlrun.api.schemas.ProjectSpec(),
Expand Down
146 changes: 142 additions & 4 deletions tests/integration/sdk_api/projects/test_project.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,16 +26,154 @@
class TestProject(tests.integration.sdk_api.base.TestMLRunIntegration):
def test_create_project(self):
project_name = "some-project"
project = mlrun.new_project(project_name)
project.save_to_db()
mlrun.new_project(project_name)
projects = mlrun.get_run_db().list_projects()
assert len(projects) == 1
assert projects[0].metadata.name == project_name

def test_create_project_failure_already_exists(self):
project_name = "some-project"
mlrun.new_project(project_name)

with pytest.raises(mlrun.errors.MLRunConflictError) as exc:
mlrun.new_project(project_name)
assert (
f"Project with name {project_name} already exists. Use overwrite=True to overwrite the existing project."
in str(exc.value)
)

def test_overwrite_project(self):
project_name = "some-project"

# verify overwrite does not fail if project does not exist
project = mlrun.new_project(project_name, overwrite=True)
db = mlrun.get_run_db()

# create several functions with several tags
labels = {
"name": "value",
"name2": "value2",
}
function = {
"test": "function",
"metadata": {"labels": labels},
"spec": {"asd": "asdasd"},
"status": {"bla": "blabla"},
}
function_names = ["function_name_1", "function_name_2", "function_name_3"]
function_tags = ["some_tag", "some_tag2", "some_tag3"]
for function_name in function_names:
for function_tag in function_tags:
db.store_function(
function,
function_name,
project.metadata.name,
tag=function_tag,
versioned=True,
)

# create several artifacts
artifact = {
"test": "artifact",
"labels": labels,
"status": {"bla": "blabla"},
}
artifact_keys = ["artifact_key_1", "artifact_key_2", "artifact_key_3"]
for artifact_key in artifact_keys:
db.store_artifact(
artifact_key,
artifact,
"some_uid",
tag="some_tag",
project=project.metadata.name,
)

projects = db.list_projects()
assert len(projects) == 1
assert projects[0].metadata.name == project_name

# verify artifacts and functions were created
project_artifacts = project.list_artifacts()
loaded_project_artifacts = projects[0].list_artifacts()
assert len(project_artifacts) == len(artifact_keys)
assert len(project_artifacts) == len(loaded_project_artifacts)
assert project_artifacts == loaded_project_artifacts

project_functions = project.list_functions()
loaded_project_functions = projects[0].list_functions()
assert len(project_functions) == len(function_names) * len(function_tags)
assert len(project_functions) == len(loaded_project_functions)

loaded_project_functions_dicts = [
func.to_dict() for func in loaded_project_functions
]
assert all(
[
func.to_dict() in loaded_project_functions_dicts
for func in project_functions
]
)

old_creation_time = projects[0].metadata.created

mlrun.new_project(project_name, overwrite=True)
projects = db.list_projects()
assert len(projects) == 1
assert projects[0].metadata.name == project_name

# ensure cascade deletion strategy
assert projects[0].list_artifacts() == []
assert projects[0].list_functions() is None
assert projects[0].metadata.created > old_creation_time

def test_overwrite_empty_project(self):
project_name = "some-project"

mlrun.new_project(project_name)
db = mlrun.get_run_db()

projects = db.list_projects()
assert len(projects) == 1
assert projects[0].metadata.name == project_name

assert projects[0].list_artifacts() == []
assert projects[0].list_functions() is None
old_creation_time = projects[0].metadata.created

# overwrite empty project
mlrun.new_project(project_name, overwrite=True)
projects = db.list_projects()
assert len(projects) == 1
assert projects[0].metadata.name == project_name

assert projects[0].list_artifacts() == []
assert projects[0].list_functions() is None
assert projects[0].metadata.created > old_creation_time

def test_overwrite_project_failure(self):
project_name = "some-project"

mlrun.new_project(project_name)
db = mlrun.get_run_db()

projects = db.list_projects()
assert len(projects) == 1
assert projects[0].metadata.name == project_name
old_creation_time = projects[0].metadata.created

# overwrite with invalid from_template value
with pytest.raises(ValueError):
mlrun.new_project(project_name, from_template="bla", overwrite=True)

# ensure project was not deleted
projects = db.list_projects()
assert len(projects) == 1
assert projects[0].metadata.name == project_name
assert projects[0].metadata.created == old_creation_time

def test_load_project_from_db(self):
project_name = "some-project"
project = mlrun.new_project(project_name)
project.save_to_db()
mlrun.new_project(project_name)
mlrun.load_project(".", f"db://{project_name}")

def test_load_project_with_save(self):
Expand Down

0 comments on commit 23f0e5f

Please sign in to comment.