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(service): improve formatting for migrationscheck response #2122

Merged
merged 23 commits into from
Nov 15, 2021
Merged
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
99b1f3d
feat(service): improve formatting or migrationscheck response
Panaetius Jun 2, 2021
39bfc40
Merge branch 'master' into 2048-finegrained-migration-information
Panaetius Jun 8, 2021
002062d
Merge branch 'master' into 2048-finegrained-migration-information
Panaetius Jun 10, 2021
8fa15df
chore: enable extra values on deploy
rokroskar Jun 14, 2021
8911d57
fix(service): do not require json on get
rokroskar Jun 14, 2021
9b491ae
Merge branch 'master' into 2048-finegrained-migration-information
Panaetius Jun 16, 2021
d9f4bb6
Merge branch 'master' into 2048-finegrained-migration-information
Panaetius Jun 17, 2021
5810c6b
Merge branch 'master' into 2048-finegrained-migration-information
Panaetius Jun 21, 2021
434705f
Merge branch 'master' into 2048-finegrained-migration-information
Panaetius Jun 21, 2021
259ec8c
Merge branch 'master' into 2048-finegrained-migration-information
Panaetius Jun 25, 2021
eb37031
update documentation
Panaetius Jul 5, 2021
748daac
Merge branch 'master' into 2048-finegrained-migration-information
Panaetius Jul 5, 2021
0368557
Merge branch 'master' into 2048-finegrained-migration-information
Panaetius Sep 16, 2021
795e8a3
fix merge problems
Panaetius Sep 16, 2021
fdb01c8
Merge branch 'master' into 2048-finegrained-migration-information
Panaetius Sep 17, 2021
509b2e3
Merge branch 'master' into 2048-finegrained-migration-information
Panaetius Nov 1, 2021
d8d6921
change new endpoint to api version 1.0
Panaetius Nov 1, 2021
f99fb98
fix IT
Panaetius Nov 1, 2021
51f62b2
Merge branch 'master' into 2048-finegrained-migration-information
Panaetius Nov 5, 2021
c915605
Merge branch 'master' into 2048-finegrained-migration-information
rokroskar Nov 9, 2021
5b57b8a
address PR comments
Panaetius Nov 15, 2021
92cf715
Merge branch 'master' into 2048-finegrained-migration-information
Panaetius Nov 15, 2021
7dd20c7
Merge branch 'master' into 2048-finegrained-migration-information
Panaetius Nov 15, 2021
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
2 changes: 2 additions & 0 deletions .github/workflows/acceptance-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ jobs:
renku-notebooks: ${{ steps.deploy-comment.outputs.renku-notebooks}}
renku-ui: ${{ steps.deploy-comment.outputs.renku-ui}}
test-enabled: ${{ steps.deploy-comment.outputs.test-enabled}}
extra-values: ${{ steps.deploy-comment.outputs.extra-values}}
steps:
- id: deploy-comment
uses: SwissDataScienceCenter/renku-actions/check-pr-description@v0.2.0
Expand Down Expand Up @@ -64,6 +65,7 @@ jobs:
renku_graph: "${{ needs.check-deploy.outputs.renku-graph }}"
renku_notebooks: "${{ needs.check-deploy.outputs.renku-notebooks }}"
renku_ui: "${{ needs.check-deploy.outputs.renku-ui }}"
extra_values: "${{ needs.check-deploy.outputs.extra-values }}"
- name: Check existing renkubot comment
uses: peter-evans/find-comment@v1
id: findcomment
Expand Down
38 changes: 3 additions & 35 deletions renku/cli/migrate.py
Original file line number Diff line number Diff line change
Expand Up @@ -121,42 +121,10 @@ def migrate(check, skip_template_update, skip_docker_update, strict):
@click.command(hidden=True)
def migrationscheck():
"""Check status of the project and current renku-python version."""
from renku.core.commands.migrate import migrations_check, migrations_versions

latest_version, project_version = migrations_versions().build().execute().output
(
migration_required,
project_supported,
template_update_possible,
current_template_version,
latest_template_version,
template_source,
template_ref,
template_id,
automated_update,
docker_update_possible,
) = (
migrations_check().lock_project().build().execute().output
)
from renku.core.commands.migrate import migrations_check

click.echo(
json.dumps(
{
"latest_version": latest_version,
"project_version": project_version,
"migration_required": migration_required,
"project_supported": project_supported,
"template_update_possible": template_update_possible,
"current_template_version": str(current_template_version),
"latest_template_version": str(latest_template_version),
"template_source": template_source,
"template_ref": template_ref,
"template_id": template_id,
"automated_update": automated_update,
"docker_update_possible": docker_update_possible,
}
)
)
result = migrations_check().lock_project().build().execute().output
click.echo(json.dumps(result))


@click.command(hidden=True)
Expand Down
108 changes: 81 additions & 27 deletions renku/core/commands/migrate.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@
from renku.core.management.command_builder.command import Command
from renku.core.management.interface.client_dispatcher import IClientDispatcher
from renku.core.management.migrate import (
SUPPORTED_PROJECT_VERSION,
_get_project_version,
Panaetius marked this conversation as resolved.
Show resolved Hide resolved
is_docker_update_possible,
is_migration_required,
is_project_unsupported,
Expand All @@ -45,38 +47,25 @@ def migrations_check():

@inject.autoparams()
def _migrations_check(client_dispatcher: IClientDispatcher):
"""Check migration status of project."""

client = client_dispatcher.current_client

template_update_possible, current_version, new_version = is_template_update_possible()
core_version, latest_version = _migrations_versions()

try:
template_source = client.project.template_source
template_ref = client.project.template_ref
template_id = client.project.template_id
automated_update = bool(client.project.automated_update)
except ValueError:
template_source = None
template_ref = None
template_id = None
automated_update = False

return (
is_migration_required(),
not is_project_unsupported(),
template_update_possible,
current_version,
new_version,
template_source,
template_ref,
template_id,
automated_update,
is_docker_update_possible(),
)
return {
"project_supported": not is_project_unsupported(),
"core_renku_version": core_version,
"project_renku_version": latest_version,
"core_compatibility_status": _metadata_migration_check(client),
"dockerfile_renku_status": _dockerfile_migration_check(client),
"template_status": _template_migration_check(client),
}


def migrations_versions():
"""Return a command to get source and destination migration versions."""
return Command().command(_migrations_versions).lock_project().with_database()
Panaetius marked this conversation as resolved.
Show resolved Hide resolved
return Command().command(_migrations_versions).lock_project()


@inject.autoparams()
Expand All @@ -97,6 +86,71 @@ def _migrations_versions(client_dispatcher: IClientDispatcher):
return __version__, latest_agent


def template_migration_check():
"""Return a command for a template migrations check."""
return Command().command(_template_migration_check)


def _template_migration_check(client):
"""Return template migration status."""
newer_template_available, current_version, new_version = is_template_update_possible()

try:
template_source = client.project.template_source
template_ref = client.project.template_ref
template_id = client.project.template_id
automated_update = bool(client.project.automated_update)
except ValueError:
template_source = None
template_ref = None
template_id = None
automated_update = False

return {
"automated_template_update": automated_update,
"newer_template_available": newer_template_available,
"project_template_version": current_version,
"latest_template_version": new_version,
"template_source": template_source,
"template_ref": template_ref,
"template_id": template_id,
}


def dockerfile_migration_check():
"""Return a command for a Dockerfile migrations check."""
return Command().command(_dockerfile_migration_check)


def _dockerfile_migration_check(client):
"""Return Dockerfile migration status."""
from renku import __version__

automated_dockerfile_update, newer_renku_available, dockerfile_renku_version = is_docker_update_possible()

return {
"automated_dockerfile_update": automated_dockerfile_update,
"newer_renku_available": newer_renku_available,
"dockerfile_renku_version": dockerfile_renku_version,
"latest_renku_version": __version__,
}


def metadata_migration_check():
"""Return a command for a metadata migrations check."""
return Command().command(_metadata_migration_check)


def _metadata_migration_check(client):
"""Return metadata migration status."""

return {
"migration_required": is_migration_required(),
"project_metadata_version": _get_project_version(),
"current_metadata_version": SUPPORTED_PROJECT_VERSION,
}


def migrate_project():
"""Return a command to migrate all project's entities."""
return Command().command(_migrate_project).lock_project().require_clean().with_database(write=True)
Expand Down Expand Up @@ -140,11 +194,11 @@ def _check_project(client_dispatcher: IClientDispatcher):

status = 0

if is_template_update_possible():
if is_template_update_possible()[0]:
status |= TEMPLATE_UPDATE_POSSIBLE
if client.project.automated_update:
status |= AUTOMATED_TEMPLATE_UPDATE_SUPPORTED
if is_docker_update_possible():
if is_docker_update_possible()[0]:
status |= DOCKERFILE_UPDATE_POSSIBLE

if is_migration_required():
Expand Down
18 changes: 9 additions & 9 deletions renku/core/management/migrate.py
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ def migrate(

if not skip_docker_update:
try:
docker_updated = _update_dockerfile()
docker_updated, _, _ = _update_dockerfile()
except DockerfileUpdateError:
raise
except (Exception, BaseException) as e:
Expand Down Expand Up @@ -192,13 +192,13 @@ def _update_template(client_dispatcher: IClientDispatcher, project_gateway: IPro
template_version = pkg_resources.parse_version(template_version)
current_version = pkg_resources.parse_version(project.template_version)
if template_version <= current_version:
return False, project.template_version, current_version
return False, str(project.template_version), str(current_version)
else:
if template_version == project.template_version:
return False, project.template_version, template_version
return False, str(project.template_version), str(template_version)

if check_only:
return True, project.template_version, template_version
return True, str(project.template_version), str(template_version)

communication.echo("Updating project from template...")

Expand Down Expand Up @@ -298,7 +298,7 @@ def _update_dockerfile(client_dispatcher: IClientDispatcher, check_only=False):
client = client_dispatcher.current_client

if not client.docker_path.exists():
return False
return False, None, None

communication.echo("Updating dockerfile...")

Expand All @@ -309,18 +309,18 @@ def _update_dockerfile(client_dispatcher: IClientDispatcher, check_only=False):
m = re.search(r"^ARG RENKU_VERSION=(\d+\.\d+\.\d+)$", dockercontent, flags=re.MULTILINE)
if not m:
if check_only:
return False
return False, None, None
raise DockerfileUpdateError(
"Couldn't update renku-python version in Dockerfile, as it doesn't contain an 'ARG RENKU_VERSION=...' line."
)

docker_version = pkg_resources.parse_version(m.group(1))

if docker_version >= current_version:
return False
return True, False, str(docker_version)

if check_only:
return True
return True, True, str(docker_version)

dockercontent = re.sub(
r"^ARG RENKU_VERSION=\d+\.\d+\.\d+$", f"ARG RENKU_VERSION={__version__}", dockercontent, flags=re.MULTILINE
Expand All @@ -331,7 +331,7 @@ def _update_dockerfile(client_dispatcher: IClientDispatcher, check_only=False):

communication.echo("Updated dockerfile.")

return True
return True, False, str(current_version)


@inject.autoparams()
Expand Down
38 changes: 3 additions & 35 deletions renku/service/controllers/cache_migrations_check.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
# limitations under the License.
"""Renku service migrations check controller."""

from renku.core.commands.migrate import migrations_check, migrations_versions
from renku.core.commands.migrate import migrations_check
from renku.service.controllers.api.abstract import ServiceCtrl
from renku.service.controllers.api.mixins import RenkuOperationMixin
from renku.service.serializers.cache import ProjectMigrationCheckRequest, ProjectMigrationCheckResponseRPC
Expand All @@ -42,40 +42,8 @@ def context(self):

def renku_op(self):
"""Renku operation for the controller."""
latest_version, project_version = migrations_versions().build().execute().output

# TODO: This API design should be improved. Next 2 lines could probably be combined to 1, ie.
# return migrations_check().build().execute().output
(
migration_required,
project_supported,
template_update_possible,
current_template_version,
latest_template_version,
template_source,
template_ref,
template_id,
automated_update,
docker_update_possible,
) = (
migrations_check().build().execute().output
)

return {
"migration_required": migration_required,
"template_update_possible": template_update_possible,
"current_template_version": current_template_version,
"latest_template_version": latest_template_version,
"template_source": template_source,
"template_ref": template_ref,
"template_id": template_id,
"automated_template_update": automated_update,
"docker_update_possible": docker_update_possible,
"project_supported": project_supported,
"project_version": project_version,
"latest_version": latest_version,
}
return migrations_check().build().execute().output

def to_response(self):
"""Execute controller flow and serialize to service response."""
return result_response(MigrationsCheckCtrl.RESPONSE_SERIALIZER, self.execute_op())
return result_response(self.RESPONSE_SERIALIZER, self.execute_op())
2 changes: 1 addition & 1 deletion renku/service/controllers/graph_export.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ def renku_op(self):
"""Renku operation for the controller."""
result = migrations_check().build().execute().output

if not result[1]:
if not result["project_supported"]:
raise RenkuException("project not supported")

callback_payload = {
Expand Down