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.

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

feat(core): consistent template versioning #2763

Merged
merged 4 commits into from Mar 24, 2022
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 2 additions & 1 deletion Makefile
Expand Up @@ -22,7 +22,7 @@ DOCKER_PREFIX:=${DOCKER_REGISTRY}$(DOCKER_REPOSITORY)
GIT_MASTER_HEAD_SHA:=$(shell git rev-parse --short --verify HEAD)

TEMPLATE_URL:=https://github.com/SwissDataScienceCenter/renku-project-template
TEMPLATE_REFERENCE:=0.3.0
TEMPLATE_REFERENCE:=$(shell sed -n -E 's/^__template_version__ = "([^"]+)"/\1/p' renku/version.py)
TEMPLATE_DIR:=renku/templates/

.PHONY: service cli docker-tag docker-push docker-login
Expand All @@ -47,6 +47,7 @@ cli:
docker build -f Dockerfile -t $(DOCKER_PREFIX)renku-python:`git rev-parse --short HEAD` --build-arg CLEAN_INSTALL=1 .

download-templates:
@[ "${TEMPLATE_REFERENCE}" ] || ( echo "__template_version__ is not set"; exit 1 )
@echo "Downloading templates"
rm -rf $(TEMPLATE_DIR)
mkdir -p $(TEMPLATE_DIR)
Expand Down
4 changes: 2 additions & 2 deletions renku/__init__.py
Expand Up @@ -19,6 +19,6 @@

from __future__ import absolute_import, print_function

from renku.version import __version__
from renku.version import __template_version__, __version__

__all__ = ("__version__",)
__all__ = ("__template_version__", "__version__")
2 changes: 1 addition & 1 deletion renku/cli/dataset.py
Expand Up @@ -1000,7 +1000,7 @@ def update(names, creators, include, exclude, ref, delete, external, no_external
def get_dataset_files(records):
from renku.core.commands.format.tabulate import tabulate

columns = {"path": ("path", None), "dataset": ("dataset.name", None), "external": ("external", None)}
columns = {"path": ("path", None), "dataset": ("dataset.name", "dataset"), "external": ("external", None)}
return tabulate(collection=records, columns="path,dataset,external", columns_mapping=columns)

if not datasets and not dataset_files:
Expand Down
2 changes: 1 addition & 1 deletion renku/core/commands/view_model/template.py
Expand Up @@ -73,7 +73,7 @@ def from_template(cls, template: Template) -> "TemplateViewModel":
parameters=template.parameters,
icon=template.icon,
immutable_files=template.immutable_files,
versions=template.get_all_versions(),
versions=template.get_all_references(),
)


Expand Down
59 changes: 24 additions & 35 deletions renku/core/management/template/template.py
Expand Up @@ -354,48 +354,42 @@ def read_valid_value(var: TemplateParameter, default_value=None):
class EmbeddedTemplates(TemplatesSource):
"""Represent templates that are bundled with Renku.

For embedded templates, ``source`` is "renku" and ``version`` is set to the installed Renku version. ``reference``
should not be set for such projects.
For embedded templates, ``source`` is "renku". In the old versioning scheme, ``version`` is set to the installed
Renku version and ``reference`` is not set. In the new scheme, both ``version`` and ``reference`` are set to the
template version.
"""

@classmethod
def fetch(cls, source: Optional[str], reference: Optional[str]) -> "EmbeddedTemplates":
"""Fetch embedded Renku templates."""
from renku import __version__

if reference and reference != "master":
raise errors.ParameterError("Templates included in renku don't support specifying a template reference")
from renku import __template_version__

path = importlib_resources.files("renku") / "templates"
with importlib_resources.as_file(path) as folder:
path = Path(folder)

return cls(path=path, source="renku", reference=None, version=str(__version__))

def is_update_available(self, id: str, reference: Optional[str], version: Optional[str]) -> Tuple[bool, str]:
"""Return True if an update is available along with the latest version of a template."""
latest_version = self.get_latest_version(id=id, reference=reference, version=version)
update_available = latest_version is not None and latest_version != version

return update_available, latest_version
return cls(path=path, source="renku", reference=__template_version__, version=__template_version__)

def get_all_versions(self, id) -> List[str]:
"""Return all available versions for a template id."""
def get_all_references(self, id) -> List[str]:
"""Return all available references for a template id."""
template_exists = any(t.id == id for t in self.templates)
return [self.version] if template_exists else []
return [self.reference] if template_exists else []

def get_latest_version(self, id: str, reference: Optional[str], version: Optional[str]) -> Optional[str]:
"""Return latest version number of a template."""
def get_latest_reference_and_version(
self, id: str, reference: Optional[str], version: Optional[str]
) -> Optional[Tuple[str, str]]:
"""Return latest reference and version number of a template."""
if version is None:
return
elif reference is None or reference != version: # Old versioning scheme
return self.reference, self.version

template_version = Version(self.version)
try:
current_version = Version(version)
except ValueError: # NOTE: version is not a valid SemVer
return str(template_version)
return self.reference, self.version
else:
return str(template_version) if current_version < template_version else version
return (self.reference, self.version) if current_version < Version(self.version) else (reference, version)

def get_template(self, id, reference: Optional[str]) -> Optional["Template"]:
"""Return all available versions for a template id."""
Expand Down Expand Up @@ -432,14 +426,7 @@ def fetch(cls, source: Optional[str], reference: Optional[str]) -> "RepositoryTe

return cls(path=path, source=source, reference=reference, version=version, repository=repository)

def is_update_available(self, id: str, reference: Optional[str], version: Optional[str]) -> Tuple[bool, str]:
"""Return True if an update is available along with the latest version of a template."""
latest_version = self.get_latest_version(id=id, reference=reference, version=version)
update_available = latest_version is not None and latest_version != reference

return update_available, latest_version

def get_all_versions(self, id) -> List[str]:
def get_all_references(self, id) -> List[str]:
"""Return a list of git tags that are valid SemVer and include a template id."""
versions = []
for tag in self.repository.tags:
Expand All @@ -454,20 +441,22 @@ def get_all_versions(self, id) -> List[str]:

return [str(v) for v in sorted(versions)]

def get_latest_version(self, id: str, reference: Optional[str], version: Optional[str]) -> Optional[str]:
"""Return latest version number of a template."""
def get_latest_reference_and_version(
self, id: str, reference: Optional[str], version: Optional[str]
) -> Optional[Tuple[str, str]]:
"""Return latest reference and version number of a template."""
if version is None:
return

tag = to_semantic_version(reference)

# NOTE: Assume that a SemVer reference is always a tag
if tag:
versions = self.get_all_versions(id=id)
return versions[-1] if len(versions) > 0 else None
references = self.get_all_references(id=id)
return (references[-1], self.version) if len(references) > 0 else None

# NOTE: Template's reference is a branch or SHA and the latest version is RepositoryTemplates' version
return self.version
return reference, self.version

def _has_template_at(self, id: str, reference: str) -> bool:
"""Return if template id is available at a reference."""
Expand Down
10 changes: 4 additions & 6 deletions renku/core/management/template/usecase.py
Expand Up @@ -70,13 +70,11 @@ def check_for_template_update(client) -> Tuple[bool, bool, Optional[str], Option
metadata = TemplateMetadata.from_client(client=client)

templates_source = fetch_templates_source(source=metadata.source, reference=metadata.reference)
latest_version = templates_source.get_latest_version(
update_available, latest_reference = templates_source.is_update_available(
id=metadata.id, reference=metadata.reference, version=metadata.version
)

update_available = latest_version is not None and latest_version != metadata.version

return update_available, metadata.allow_update, metadata.version, latest_version
return update_available, metadata.allow_update, metadata.reference, latest_reference


@inject.autoparams("client_dispatcher")
Expand Down Expand Up @@ -130,7 +128,7 @@ def update_template(

templates_source = fetch_templates_source(source=template_metadata.source, reference=template_metadata.reference)

update_available, latest_version = templates_source.is_update_available(
update_available, latest_reference = templates_source.is_update_available(
id=template_metadata.id, reference=template_metadata.reference, version=template_metadata.version
)

Expand All @@ -139,7 +137,7 @@ def update_template(

rendered_template, actions = _set_or_update_project_from_template(
templates_source=templates_source,
reference=latest_version,
reference=latest_reference,
id=template_metadata.id,
interactive=interactive,
dry_run=dry_run,
Expand Down
25 changes: 16 additions & 9 deletions renku/core/models/template.py
Expand Up @@ -60,19 +60,26 @@ def templates(self) -> List["Template"]:

return self.manifest.templates

@abstractmethod
def is_update_available(self, id: str, reference: Optional[str], version: Optional[str]) -> Tuple[bool, str]:
"""Return True if an update is available along with the latest version of a template."""
raise NotImplementedError
"""Return True if an update is available along with the latest reference of a template."""
latest = self.get_latest_reference_and_version(id=id, reference=reference, version=version)
if not latest:
return False, reference

latest_reference, latest_version = latest
update_available = latest_reference != reference or latest_version != version
return update_available, latest_reference
Copy link
Member

Choose a reason for hiding this comment

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

This would mean that if I install hypothetical version 1.3.0 and migrate, and then install hypothetical version 1.2.0 and run migration check, it'd say a new version is available and downgrade, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, EmbeddedTemplates::get_latest_reference_and_version checks if project's template version is newer than renku's template version and returns the latest of the two, so, 1.3.0 will be the latest version/reference in that case and nothing will be changed.


@abstractmethod
def get_all_versions(self, id) -> List[str]:
def get_all_references(self, id) -> List[str]:
"""Return all available versions for a template id."""
raise NotImplementedError

@abstractmethod
def get_latest_version(self, id: str, reference: Optional[str], version: Optional[str]) -> Optional[str]:
"""Return latest version number of a template."""
def get_latest_reference_and_version(
self, id: str, reference: Optional[str], version: Optional[str]
) -> Optional[Tuple[str, str]]:
"""Return latest reference and version number of a template."""
raise NotImplementedError

@abstractmethod
Expand Down Expand Up @@ -220,9 +227,9 @@ def templates_source(self, templates_source: TemplatesSource):
self.version = templates_source.version
self.path = templates_source.path / self.id

def get_all_versions(self) -> List[str]:
"""Return all available versions for the template."""
return self.templates_source.get_all_versions(self.id)
def get_all_references(self) -> List[str]:
"""Return all available references for the template."""
return self.templates_source.get_all_references(self.id)

def validate(self, skip_files):
"""Validate a template."""
Expand Down
1 change: 1 addition & 0 deletions renku/version.py
Expand Up @@ -25,6 +25,7 @@
from importlib_metadata import distribution

__version__ = "0.0.0"
__template_version__ = "0.3.0"


def is_release():
Expand Down
8 changes: 6 additions & 2 deletions tests/cli/test_template.py
Expand Up @@ -80,7 +80,7 @@ def test_template_show(isolated_runner):
result = isolated_runner.invoke(cli, command + ["R-minimal"])

assert 0 == result.exit_code, format_result_exception(result)
assert "Name: Basic R (4.1.2) Project" in result.output
assert re.search("^Name: Basic R (.*) Project$", result.output, re.MULTILINE) is not None
finally:
sys.argv = argv

Expand All @@ -90,7 +90,7 @@ def test_template_show_no_id(runner, client):
result = runner.invoke(cli, ["template", "show"])

assert 0 == result.exit_code, format_result_exception(result)
assert "Name: Basic Python (3.9) Project" in result.output
assert re.search("^Name: Basic Python (.*) Project$", result.output, re.MULTILINE) is not None


def test_template_show_no_id_outside_project(isolated_runner):
Expand Down Expand Up @@ -136,11 +136,15 @@ def test_template_set_failure(runner, client, client_database_injection_manager)

def test_template_set(runner, client, client_database_injection_manager):
"""Test setting a new template in a project."""
from renku.version import __template_version__

result = runner.invoke(cli, ["template", "set", "--force", "R-minimal"])

assert 0 == result.exit_code, format_result_exception(result)
with client_database_injection_manager(client):
assert "R-minimal" == client.project.template_id
assert __template_version__ == client.project.template_version
assert __template_version__ == client.project.template_ref


def test_template_set_overwrites_modified(runner, client, client_database_injection_manager):
Expand Down
17 changes: 10 additions & 7 deletions tests/fixtures/templates.py
Expand Up @@ -32,7 +32,7 @@ def template_metadata():
"""Default template metadata."""
yield {
"__template_source__": "renku",
"__template_ref__": "master",
"__template_ref__": renku_version,
"__template_id__": "python-minimal",
"__namespace__": "",
"__repository__": "",
Expand Down Expand Up @@ -172,18 +172,21 @@ def fetch(cls, source: Optional[str], reference: Optional[str]) -> "TemplatesSou

def is_update_available(self, id: str, reference: Optional[str], version: Optional[str]) -> Tuple[bool, str]:
"""Return True if an update is available along with the latest version of a template."""
latest_version = self.get_latest_version(id=id, reference=reference, version=version)
_, latest_version = self.get_latest_reference_and_version(id=id, reference=reference, version=version)

return latest_version != version, latest_version

def get_all_versions(self, id) -> List[str]:
"""Return all available versions for a template id."""
def get_all_references(self, id) -> List[str]:
"""Return all available references for a template id."""
return [str(v) for v in self._versions]

def get_latest_version(self, id: str, reference: Optional[str], version: Optional[str]) -> Optional[str]:
"""Return latest version number of a template."""
def get_latest_reference_and_version(
self, id: str, reference: Optional[str], version: Optional[str]
) -> Optional[Tuple[str, str]]:
"""Return latest reference and version number of a template."""
_ = self.get_template(id=id, reference=reference)
return str(max(self._versions))
version = str(max(self._versions))
return version, version

def get_template(self, id, reference: Optional[str]) -> Optional[Template]:
"""Return a template at a specific reference."""
Expand Down