Skip to content

Remove CLI command configuration #88

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

Merged
merged 1 commit into from
May 19, 2023
Merged

Conversation

F-X64
Copy link
Member

@F-X64 F-X64 commented May 19, 2023

Remove unnecessary configuration option
Remove unnecessary states
Add hardcoded defaults for variables
Add proper instancetype checking for arm64 / x86 AWS images

Closes #72

Remove unnecessary configuration option
Remove unnecessary states
Add hardcoded defaults for variables
Add proper instancetype checking for arm64 / x86 AWS images
@F-X64 F-X64 requested review from major and miyunari May 19, 2023 09:45
@miyunari
Copy link
Member

Does this PR remove this? :) :
image

@F-X64
Copy link
Member Author

F-X64 commented May 19, 2023

Yeah it will be hard to properly maintain (and implement to begin with).

E.g. the machine type is specific to the selected image and can only be retrieved from external sources like this https://cloud.google.com/compute/docs/images/os-details#red_hat_enterprise_linux_rhel
(AFAIK there is no information in the image data that says which machine types are supported).

I've added sensible defaults for AWS, Azure and Google but making all of them select able or configurable would be a lot of work.

The other thing that really bothers me about this feature is that it can't pre-populate all of the fields. I don't think anyone would like to pass the key name (on the bottom of your screenshot) through our website to the AWS console.

I'm not against implementing this in a proper way but my fast-and-scrappy approach is more confusing than actually helpful (and really hard to maintain should we add more cloud provider / distros).

@miyunari
Copy link
Member

Makes sense! Thanks for the explanation! 👍

@F-X64 F-X64 merged commit 4badb97 into main May 19, 2023
@F-X64 F-X64 deleted the add-proper-aws-machine-type branch May 19, 2023 11:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The default machine type for AWS is invalid
2 participants