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.

Sign up for GitHub

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

Jump to bottom

feature: human readable cloud provider #1744

Closed
wants to merge 7 commits into from
Closed

feature: human readable cloud provider #1744

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
4cb4f86
WIP
Apr 22, 2023
e89071f
update cloud_provider name
Apr 22, 2023
7d032dc
Merge branch 'develop' into feature/human-readable-cloud-provider
iameskild May 1, 2023
16ced14
update the providerenum
May 1, 2023
264866e
Merge branch 'develop' into feature/human-readable-cloud-provider
iameskild May 4, 2023
84fa0e9
Merge branch 'develop' into feature/human-readable-cloud-provider
viniciusdc May 10, 2023
8aec8f3
Merge branch 'develop' into feature/human-readable-cloud-provider
iameskild May 18, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 9 additions & 7 deletions nebari/cli/init.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,10 @@
CHOOSE_CLOUD_PROVIDER = "https://nebari.dev/docs/get-started/deploy"


def enum_to_list(enum_cls):
return [e.value.lower() for e in enum_cls]
def enum_to_list(enum_cls, lower=True):
iameskild marked this conversation as resolved.
Show resolved Hide resolved
if lower:
return [e.value.lower() for e in enum_cls]
return [e.value for e in enum_cls]


def handle_init(inputs: InitInputs):
Expand Down Expand Up @@ -290,7 +292,6 @@ def guided_init_wizard(ctx: typer.Context, guided_init: str):
"""
qmark = " "
disable_checks = os.environ.get("NEBARI_DISABLE_INIT_CHECKS", False)

if Path("nebari-config.yaml").exists():
raise ValueError(
"A nebari-config.yaml file already exists. Please move or delete it and try again."
Expand Down Expand Up @@ -330,24 +331,25 @@ def guided_init_wizard(ctx: typer.Context, guided_init: str):
# try:
inputs.cloud_provider = questionary.select(
"Where would you like to deploy your Nebari cluster?",
choices=enum_to_list(ProviderEnum),
choices=enum_to_list(ProviderEnum, lower=False),
iameskild marked this conversation as resolved.
Show resolved Hide resolved
qmark=qmark,
).unsafe_ask()

inputs.cloud_provider = ProviderEnum(inputs.cloud_provider).name

if not disable_checks:
check_cloud_provider_creds(ctx, cloud_provider=inputs.cloud_provider)

# specific context needed when `check_project_name` is called
ctx.params["cloud_provider"] = inputs.cloud_provider

name_guidelines = """
The project name must adhere to the following requirements:
- Letters from A to Z (upper and lower case) and numbers
- Maximum accepted length of the name string is 16 characters
"""
if inputs.cloud_provider == ProviderEnum.aws.value.lower():
if inputs.cloud_provider == ProviderEnum.aws.name:
name_guidelines += "- Should NOT start with the string `aws`\n"
elif inputs.cloud_provider == ProviderEnum.azure.value.lower():
elif inputs.cloud_provider == ProviderEnum.azure.name:
name_guidelines += "- Should NOT contain `-`\n"

# PROJECT NAME
Expand Down
12 changes: 6 additions & 6 deletions nebari/schema.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,12 @@ class TerraformStateEnum(str, enum.Enum):


class ProviderEnum(str, enum.Enum):
local = "local"
existing = "existing"
do = "do"
aws = "aws"
gcp = "gcp"
azure = "azure"
local = "Local test k8s cluster (using 'kind')"
existing = "Existing k8s cluster"
do = "DigitalOcean (DO)"
aws = "Amazon Web Services (AWS)"
gcp = "Google Cloud Platform (GCP)"
azure = "Microsoft Azure (Azure)"
Comment on lines +27 to +32
Copy link
Member

@iameskild iameskild May 18, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Upon closer inspection, I don't think we want to modify this enum class directly; this is why the pytests are failing. This is the "canonical" label for each cloud provider and is being used elsewhere in the codebase to validate the schema.

Instead, inside of the nebari/cli/init.py we can include a mapping from these enum names to the "full names" of each of the cloud providers:

CLOUD_PROVIDER_FULL_NAME = {
    "Local": ProviderEnum.local.name,
    "Existing": ProviderEnum.existing.name,
    "Digital Ocean": ProviderEnum.do.name,
    "Amazon Web Services": ProviderEnum.aws.name,
    "Google Cloud Platform": ProviderEnum.gcp.name,
    "Microsoft Azure": ProviderEnum.azure.name,
}

...

        inputs.cloud_provider = questionary.select(
            "Where would you like to deploy your Nebari cluster?",
            choices=CLOUD_PROVIDER_FULL_NAME.keys(),
            qmark=qmark,
        ).unsafe_ask()

		# map the value returned by the user back into the proper enum name
        inputs.cloud_provider = CLOUD_PROVIDER_FULL_NAME.get(inputs.cloud_provider)

Copy link
Member

Choose a reason for hiding this comment

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

@shashvatshah9 Hi and thanks for contributing this PR! I'm pinging to surface this comment, would you have the time to incorporate @iameskild's suggestion? It'll be great to get this PR merged. :)

Copy link
Member

Choose a reason for hiding this comment

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

Hi @shashvatshah9, if it's alright with you, I will be wrapping up this PR later this week.

Copy link
Member

Choose a reason for hiding this comment

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

Hi @shashvatshah9, if it's alright with you, I will be wrapping up this PR later this week.

Copy link
Author

Choose a reason for hiding this comment

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

Hi @iameskild, I won't be able to look at a close look at the PR changes, so please close it if it's alright. Thanks!



class GitRepoEnum(str, enum.Enum):
Expand Down