Skip to content

Commit

Permalink
Rm "default" profile migration (#15158)
Browse files Browse the repository at this point in the history
  • Loading branch information
zzstoatzz authored Aug 30, 2024
1 parent 266557e commit ecbd6f9
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 76 deletions.
25 changes: 2 additions & 23 deletions src/prefect/cli/profile.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
from prefect.client.orchestration import ServerType, get_client
from prefect.context import use_profile
from prefect.exceptions import ObjectNotFound
from prefect.settings import Profile, ProfilesCollection
from prefect.settings import ProfilesCollection
from prefect.utilities.collections import AutoEnum

profile_app = PrefectTyper(name="profile", help="Select and manage Prefect profiles.")
Expand All @@ -38,7 +38,7 @@ def ls():
"""
List profile names.
"""
profiles = prefect.settings.load_profiles()
profiles = prefect.settings.load_profiles(include_defaults=False)
current_profile = prefect.context.get_settings_context().profile
current_name = current_profile.name if current_profile is not None else None

Expand Down Expand Up @@ -265,9 +265,6 @@ def show_profile_changes(
):
changes = []

if "default" in user_profiles:
changes.append(("migrate", "default", "ephemeral"))

for name in default_profiles.names:
if name not in user_profiles:
changes.append(("add", name))
Expand Down Expand Up @@ -320,24 +317,6 @@ def populate_defaults():
app.console.print("Operation cancelled.")
return

# Apply changes
if "default" in user_profiles:
default_settings = user_profiles["default"].settings
if "ephemeral" not in user_profiles:
user_profiles.add_profile(
Profile(name="ephemeral", settings=default_settings)
)
else:
merged_settings = {
**user_profiles["ephemeral"].settings,
**default_settings,
}
user_profiles.update_profile("ephemeral", merged_settings)

if user_profiles.active_name == "default":
user_profiles.set_active("ephemeral")
user_profiles.remove_profile("default")

for name, profile in default_profiles.items():
if name not in user_profiles:
user_profiles.add_profile(profile)
Expand Down
13 changes: 10 additions & 3 deletions src/prefect/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -2191,13 +2191,20 @@ def _write_profiles_to(path: Path, profiles: ProfilesCollection) -> None:
return path.write_text(toml.dumps(profiles.to_dict()))


def load_profiles() -> ProfilesCollection:
def load_profiles(include_defaults: bool = True) -> ProfilesCollection:
"""
Load all profiles from the default and current profile paths.
Load profiles from the current profile path. Optionally include profiles from the
default profile path.
"""
profiles = _read_profiles_from(DEFAULT_PROFILES_PATH)
default_profiles = _read_profiles_from(DEFAULT_PROFILES_PATH)

if not include_defaults:
if not PREFECT_PROFILES_PATH.value().exists():
return ProfilesCollection([])
return _read_profiles_from(PREFECT_PROFILES_PATH.value())

user_profiles_path = PREFECT_PROFILES_PATH.value()
profiles = default_profiles
if user_profiles_path.exists():
user_profiles = _read_profiles_from(user_profiles_path)

Expand Down
51 changes: 1 addition & 50 deletions tests/cli/test_profile.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
PREFECT_API_URL,
PREFECT_DEBUG_MODE,
PREFECT_PROFILES_PATH,
PREFECT_SERVER_ALLOW_EPHEMERAL_MODE,
Profile,
ProfilesCollection,
_read_profiles_from,
Expand Down Expand Up @@ -220,13 +219,6 @@ def test_using_ephemeral_server(self, profiles):
assert profiles.active_name == "ephemeral"


def test_ls_default_profiles():
# 'ephemeral' is not the current profile because we have a temporary profile in-use
# during tests

invoke_and_assert(["profile", "ls"], expected_output_contains="ephemeral")


def test_ls_additional_profiles():
# 'ephemeral' is not the current profile because we have a temporary profile in-use
# during tests
Expand All @@ -244,7 +236,6 @@ def test_ls_additional_profiles():
invoke_and_assert(
["profile", "ls"],
expected_output_contains=(
"ephemeral",
"foo",
"bar",
),
Expand All @@ -263,10 +254,7 @@ def test_ls_respects_current_from_profile_flag():

invoke_and_assert(
["--profile", "foo", "profile", "ls"],
expected_output_contains=(
"ephemeral",
"* foo",
),
expected_output_contains=("* foo",),
)


Expand All @@ -285,7 +273,6 @@ def test_ls_respects_current_from_context():
invoke_and_assert(
["profile", "ls"],
expected_output_contains=(
"ephemeral",
"foo",
"* bar",
),
Expand Down Expand Up @@ -641,41 +628,6 @@ def test_populate_defaults_no_changes_needed(self, temporary_profiles_path):

assert temporary_profiles_path.read_text() == DEFAULT_PROFILES_PATH.read_text()

def test_populate_defaults_migrates_default(self, temporary_profiles_path):
existing_profiles = ProfilesCollection(
profiles=[
Profile(name="default", settings={PREFECT_API_KEY: "test_key"}),
Profile(name="custom", settings={PREFECT_API_URL: "http://custom.url"}),
],
active="default",
)
save_profiles(existing_profiles)

invoke_and_assert(
["profile", "populate-defaults"],
user_input="y\ny", # Confirm backup and update
expected_output_contains=[
"Proposed Changes:",
"Migrate 'default' to 'ephemeral'",
"Add 'local'",
"Add 'cloud'",
f"Back up existing profiles to {temporary_profiles_path}.bak?",
f"Update profiles at {temporary_profiles_path}?",
f"Profiles updated in {temporary_profiles_path}",
],
)

new_profiles = load_profiles()
assert {"ephemeral", "local", "cloud", "test", "custom"} == set(
new_profiles.names
)
assert "default" not in new_profiles.names
assert new_profiles.active_name == "ephemeral"
assert new_profiles["ephemeral"].settings == {
PREFECT_API_KEY: "test_key",
PREFECT_SERVER_ALLOW_EPHEMERAL_MODE: "true",
}

def test_show_profile_changes(self, capsys):
default_profiles = ProfilesCollection(
profiles=[
Expand Down Expand Up @@ -707,7 +659,6 @@ def test_show_profile_changes(self, capsys):
output = captured.out

assert "Proposed Changes:" in output
assert "Migrate 'default' to 'ephemeral'" in output
assert "Add 'ephemeral'" in output
assert "Add 'local'" in output
assert "Add 'cloud'" in output

0 comments on commit ecbd6f9

Please sign in to comment.