-
Notifications
You must be signed in to change notification settings - Fork 75
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(store): promote command #2082
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -25,13 +25,14 @@ | |||||||||||||||||||||
import re | ||||||||||||||||||||||
import shutil | ||||||||||||||||||||||
import string | ||||||||||||||||||||||
import sys | ||||||||||||||||||||||
import tempfile | ||||||||||||||||||||||
import textwrap | ||||||||||||||||||||||
import typing | ||||||||||||||||||||||
import zipfile | ||||||||||||||||||||||
from collections.abc import Collection | ||||||||||||||||||||||
from operator import attrgetter | ||||||||||||||||||||||
from typing import TYPE_CHECKING, Any | ||||||||||||||||||||||
from typing import TYPE_CHECKING, Any, cast | ||||||||||||||||||||||
|
||||||||||||||||||||||
import yaml | ||||||||||||||||||||||
from craft_application import util | ||||||||||||||||||||||
|
@@ -43,11 +44,13 @@ | |||||||||||||||||||||
from craft_store.models import ResponseCharmResourceBase | ||||||||||||||||||||||
from humanize import naturalsize | ||||||||||||||||||||||
from tabulate import tabulate | ||||||||||||||||||||||
from typing_extensions import override | ||||||||||||||||||||||
|
||||||||||||||||||||||
import charmcraft.store.models | ||||||||||||||||||||||
from charmcraft import const, env, errors, parts, utils | ||||||||||||||||||||||
from charmcraft.application.commands.base import CharmcraftCommand | ||||||||||||||||||||||
from charmcraft.models import project | ||||||||||||||||||||||
from charmcraft.services.store import StoreService | ||||||||||||||||||||||
from charmcraft.store import Store | ||||||||||||||||||||||
from charmcraft.store.models import Entity | ||||||||||||||||||||||
from charmcraft.utils import cli | ||||||||||||||||||||||
|
@@ -830,6 +833,141 @@ def run(self, parsed_args): | |||||||||||||||||||||
emit.message(msg.format(*args)) | ||||||||||||||||||||||
|
||||||||||||||||||||||
|
||||||||||||||||||||||
class PromoteCommand(CharmcraftCommand): | ||||||||||||||||||||||
"""Promote a charm in the Store.""" | ||||||||||||||||||||||
|
||||||||||||||||||||||
name = "promote" | ||||||||||||||||||||||
help_msg = "Promote a charm from one channel to another on Charmhub." | ||||||||||||||||||||||
overview = "TODO" | ||||||||||||||||||||||
|
||||||||||||||||||||||
@override | ||||||||||||||||||||||
def needs_project(self, parsed_args: argparse.Namespace) -> bool: | ||||||||||||||||||||||
if parsed_args.name is None: | ||||||||||||||||||||||
emit.progress("Inferring name from project file.", permanent=True) | ||||||||||||||||||||||
return True | ||||||||||||||||||||||
return False | ||||||||||||||||||||||
|
||||||||||||||||||||||
@override | ||||||||||||||||||||||
def fill_parser(self, parser: "ArgumentParser") -> None: | ||||||||||||||||||||||
parser.add_argument( | ||||||||||||||||||||||
"--name", | ||||||||||||||||||||||
help="the name of the charm to promote. If not specified, the name will be inferred from the charm in the current directory.", | ||||||||||||||||||||||
) | ||||||||||||||||||||||
parser.add_argument( | ||||||||||||||||||||||
"--from-channel", | ||||||||||||||||||||||
metavar="from-channel", | ||||||||||||||||||||||
help="the channel to promote from", | ||||||||||||||||||||||
required=True, | ||||||||||||||||||||||
) | ||||||||||||||||||||||
parser.add_argument( | ||||||||||||||||||||||
"--to-channel", | ||||||||||||||||||||||
metavar="to-channel", | ||||||||||||||||||||||
help="the channel to promote to", | ||||||||||||||||||||||
required=True, | ||||||||||||||||||||||
) | ||||||||||||||||||||||
parser.add_argument( | ||||||||||||||||||||||
"--yes", | ||||||||||||||||||||||
default=False, | ||||||||||||||||||||||
action="store_true", | ||||||||||||||||||||||
help="Answer yes to all questions.", | ||||||||||||||||||||||
) | ||||||||||||||||||||||
|
||||||||||||||||||||||
@override | ||||||||||||||||||||||
def run(self, parsed_args: argparse.Namespace) -> int | None: | ||||||||||||||||||||||
emit.progress( | ||||||||||||||||||||||
f"{self._app.name} {self.name} does not have a stable CLI interface. " | ||||||||||||||||||||||
"Use with caution in scripts.", | ||||||||||||||||||||||
permanent=True, | ||||||||||||||||||||||
) | ||||||||||||||||||||||
store = cast(StoreService, self._services.get("store")) | ||||||||||||||||||||||
|
||||||||||||||||||||||
name = parsed_args.name or self._services.project.name | ||||||||||||||||||||||
|
||||||||||||||||||||||
# Check snapcraft for equiv logic | ||||||||||||||||||||||
from_channel = charmcraft.store.models.ChannelData.from_str( | ||||||||||||||||||||||
parsed_args.from_channel | ||||||||||||||||||||||
) | ||||||||||||||||||||||
to_channel = charmcraft.store.models.ChannelData.from_str( | ||||||||||||||||||||||
parsed_args.to_channel | ||||||||||||||||||||||
) | ||||||||||||||||||||||
if None in (from_channel.track, to_channel.track): | ||||||||||||||||||||||
package_metadata = store.get_package_metadata(name) | ||||||||||||||||||||||
default_track = package_metadata.default_track | ||||||||||||||||||||||
if from_channel.track is None: | ||||||||||||||||||||||
from_channel = dataclasses.replace(from_channel, track=default_track) | ||||||||||||||||||||||
if to_channel.track is None: | ||||||||||||||||||||||
to_channel = dataclasses.replace(to_channel, track=default_track) | ||||||||||||||||||||||
Comment on lines
+886
to
+899
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure about this logic, shouldn't we just make it explicit and error out if track is missing ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If both are missing, will it try to promote from default to default? |
||||||||||||||||||||||
|
||||||||||||||||||||||
if to_channel == from_channel: | ||||||||||||||||||||||
raise CraftError( | ||||||||||||||||||||||
"Cannot promote from a channel to the same channel.", | ||||||||||||||||||||||
retcode=64, # Replace with os.EX_USAGE once we drop Windows. | ||||||||||||||||||||||
) | ||||||||||||||||||||||
if to_channel.risk > from_channel.risk: | ||||||||||||||||||||||
dariuszd21 marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||
command_parts = [ | ||||||||||||||||||||||
self._app.name, | ||||||||||||||||||||||
f"--from-channel={to_channel.name}", | ||||||||||||||||||||||
f"--to-channel={from_channel.name}", | ||||||||||||||||||||||
self.name, | ||||||||||||||||||||||
] | ||||||||||||||||||||||
command = " ".join(command_parts) | ||||||||||||||||||||||
raise CraftError( | ||||||||||||||||||||||
f"Target channel ({to_channel.name}) must be lower risk " | ||||||||||||||||||||||
f"than the source channel ({from_channel.name}).", | ||||||||||||||||||||||
resolution=f"Did you mean: {command}", | ||||||||||||||||||||||
) | ||||||||||||||||||||||
if to_channel.track != from_channel.track: | ||||||||||||||||||||||
if not parsed_args.yes and not utils.confirm_with_user( | ||||||||||||||||||||||
"Did you mean to promote to a different track? (from " | ||||||||||||||||||||||
f"{from_channel.track} to {to_channel.track})", | ||||||||||||||||||||||
): | ||||||||||||||||||||||
emit.message("Cancelling.") | ||||||||||||||||||||||
return 64 # Replace with os.EX_USAGE once we drop Windows. | ||||||||||||||||||||||
Comment on lines
+920
to
+925
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we shouldn't allow non-interactive promotion between tracks at all |
||||||||||||||||||||||
|
||||||||||||||||||||||
candidates = store.get_revisions_on_channel(name, from_channel.name) | ||||||||||||||||||||||
|
||||||||||||||||||||||
def get_base_strings(bases): | ||||||||||||||||||||||
if bases is None: | ||||||||||||||||||||||
return "" | ||||||||||||||||||||||
return ",".join( | ||||||||||||||||||||||
f"{base.name}@{base.channel}:{base.architecture}" for base in bases | ||||||||||||||||||||||
) | ||||||||||||||||||||||
|
||||||||||||||||||||||
presentable_candidates = [ | ||||||||||||||||||||||
{ | ||||||||||||||||||||||
"Revision": info["revision"], | ||||||||||||||||||||||
"Platforms": get_base_strings(info["bases"]), | ||||||||||||||||||||||
"Resource revisions": ", ".join( | ||||||||||||||||||||||
f"{res['name']}: {res['revision']}" for res in info["resources"] | ||||||||||||||||||||||
), | ||||||||||||||||||||||
} | ||||||||||||||||||||||
for info in sorted(candidates, key=lambda x: x["revision"]) | ||||||||||||||||||||||
] | ||||||||||||||||||||||
emit.progress( | ||||||||||||||||||||||
f"The following revisions are on the {from_channel.name} channel:", | ||||||||||||||||||||||
permanent=True, | ||||||||||||||||||||||
) | ||||||||||||||||||||||
with emit.pause(): | ||||||||||||||||||||||
print( | ||||||||||||||||||||||
tabulate(presentable_candidates, tablefmt="plain", headers="keys"), | ||||||||||||||||||||||
file=sys.stderr, | ||||||||||||||||||||||
) | ||||||||||||||||||||||
Comment on lines
+950
to
+954
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Any particular reason why it can't be a progress as well (beside not logging it) ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would it work this way ? It seems to be a bit more consistent and does not leak the
Suggested change
|
||||||||||||||||||||||
if not parsed_args.yes and not utils.confirm_with_user( | ||||||||||||||||||||||
f"Do you want to promote these revisions to the {to_channel.name} channel?" | ||||||||||||||||||||||
): | ||||||||||||||||||||||
emit.message("Channel promotion cancelled.") | ||||||||||||||||||||||
return 1 | ||||||||||||||||||||||
lengau marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||
|
||||||||||||||||||||||
promotion_results = store.release_promotion_candidates( | ||||||||||||||||||||||
name, to_channel.name, candidates | ||||||||||||||||||||||
) | ||||||||||||||||||||||
|
||||||||||||||||||||||
emit.message( | ||||||||||||||||||||||
f"{len(promotion_results)} revisions promoted from {from_channel.name} to {to_channel.name}" | ||||||||||||||||||||||
) | ||||||||||||||||||||||
return 0 | ||||||||||||||||||||||
|
||||||||||||||||||||||
|
||||||||||||||||||||||
class PromoteBundleCommand(CharmcraftCommand): | ||||||||||||||||||||||
"""Promote a bundle in the Store.""" | ||||||||||||||||||||||
|
||||||||||||||||||||||
|
@@ -2098,7 +2236,6 @@ def run(self, parsed_args: argparse.Namespace) -> int: | |||||||||||||||||||||
resolution="Pass a valid container transport string.", | ||||||||||||||||||||||
) | ||||||||||||||||||||||
emit.debug(f"Using source path {source_path!r}") | ||||||||||||||||||||||
|
||||||||||||||||||||||
emit.progress("Inspecting source image") | ||||||||||||||||||||||
image_metadata = image_service.inspect(source_path) | ||||||||||||||||||||||
|
||||||||||||||||||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think few words may be helpful as it'll land in the docs 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will do - I've asked for your second review for other things ITMT