feature: human readable cloud provider

[shashvatshah9]

Reference Issues or PRs

Issue targetted issue-1708
Updated the ProviderEnum values.
Changing the value raised some other issues, so updated the enum_to_list() to add a case to avoid to lower() a string.
Also updated a few lines where comparison was made with Enum values

What does this implement/fix?

• [x ] Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds a feature)
• [x ] Breaking change (fix or feature that would cause existing features not to work as expected)
• Documentation Update
• Code style update (formatting, renaming)
• Refactoring (no functional changes, no API changes)
• Build related changes
• Other (please describe):

Testing

• [ x] Did you test the pull request locally?
• Did you add new tests?

Any other comments?

Some tests are failing on my local, and it needs to be checked if there is some value hardcoding in tests. Need help on this.

[iameskild]

Thank you for the contribution @shashvatshah9 :)

[dharhas]

@shashvatshah9 @iameskild

I think we should make the following changes before we merge:

1. We are not using lower=False so we should remove that change
2. I have some suggestions on the new text.

Instead of

local = "local"
existing = "existing K8s cluster"
do = "DigitalOcean"
aws = "Amazon Web Services (AWS)"
gcp = "Google Cloud Platform (GCP)"
azure = "Microsoft Azure"

Lets clean up slightly for consistency

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)"

[iameskild]

Hey @shashvatshah9, this PR looks like it's really close to being done. Do you have time to wrap this up? Thanks :)

[shashvatshah9]

Hey @shashvatshah9, this PR looks like it's really close to being done. Do you have time to wrap this up? Thanks :)

Updated the ProviderEnum with the latest string values, but tests are failing due to missing env variable in local NEBARI_DISABLE_INIT_CHECKS. So need some help here with test configuration.

FAILED tests/test_dependencies.py::test_build_by_conda_forge - subprocess.CalledProcessError: Command '['conda', 'build', PosixPath('/private/tmp/pytest-of-root/pytest-0/te...
FAILED tests/test_schema.py::test_schema[setup_fixture0] - KeyError: 'provider'
FAILED tests/test_schema.py::test_schema[setup_fixture1] - KeyError: 'provider'
FAILED tests/test_schema.py::test_schema[setup_fixture2] - KeyError: 'provider'
FAILED tests/test_schema.py::test_schema[setup_fixture3] - KeyError: 'provider'
FAILED tests/test_upgrade.py::test_upgrade_4_0[./qhub-config-yaml-files-for-upgrade/qhub-config-do-310.yaml-False-False] - KeyError: 'provider'
FAILED tests/test_upgrade.py::test_upgrade_4_0[./qhub-config-yaml-files-for-upgrade/qhub-config-do-310-customauth.yaml-False-True] - KeyError: 'provider'
FAILED tests/test_upgrade.py::test_upgrade_4_0[./qhub-config-yaml-files-for-upgrade/qhub-config-do-310-customauth.yaml-True-False] - KeyError: 'provider'

[shashvatshah9]

Also, we do need lower=False in enum_to_list() method call. It not an ideal way of doing things, but somehow that's how the comparisons are being done with enums in this codebase

[iameskild]

Hi @shashvatshah9 sorry for all of the back and forth but I commented below that I think we ought to keep the ProviderEnum unmodified; I provided a potential alternative.

Thanks again for your contribution! We appreciate you taking the time to work with us :)

[iameskild]

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)
        

[pavithraes]

@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. :)

[iameskild]

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

[iameskild]

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

[shashvatshah9]

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!

[iameskild]

Hi @shashvatshah9, we will be closing this PR for now. There is a large refactor in the work that will incorporate the changes you proposed so thank you for your contributions :)