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.

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

Added logic to load locations and VM sku's from json files #444

Merged
merged 14 commits into from
Nov 8, 2022

Conversation

fireblade95402
Copy link
Contributor

@fireblade95402 fireblade95402 commented Oct 31, 2022

PR Summary

This change loads the vm sku's from a json file by location that is generated by a bash script in the build.yml action. Plus, loads the locations from another locations.json file.

The genertation of toe sku's is driven from a locations.json and skuFamilies.json files, which needs to be manually updated at this point in time.

The current locations and skufamilies files are aligned to the current list from main, excluding UKSouth2, which wasn't found in the API calls to fetch the sku's.

The screen has changed to also a custom sku to be entered and sku name has been added to the dropdown

Logic has also been added to handle the 3 compute types on the screen, but the IO Optimized and GPU Workloads are disabled for now.

PR Checklist

  • PR has a meaningful title
  • Summarized changes
  • This PR is ready to merge and is not Work in Progress
  • Link to a filed issue
  • Screenshot of UI changes (if PR includes UI changes)
    image

@fireblade95402
Copy link
Contributor Author

@Gordonby I made an attemp to add the bash script to the build.yml process. I wasn't sure if if was correct or not. Plus, not too sure how to test it.

@fireblade95402 fireblade95402 marked this pull request as ready for review October 31, 2022 17:22
@fireblade95402 fireblade95402 linked an issue Oct 31, 2022 that may be closed by this pull request
@Gordonby
Copy link
Collaborator

Gordonby commented Nov 4, 2022

This looks really good @fireblade95402
Let me spin it up in a codespace

@Gordonby
Copy link
Collaborator

Gordonby commented Nov 8, 2022

before

image

after

image

@Gordonby
Copy link
Collaborator

Gordonby commented Nov 8, 2022

@fireblade95402 - i'm seeing this problem when running the script. Perhaps we need more error handling, or debug output?

I'm testing from a codespace in this repo.

image

@fireblade95402
Copy link
Contributor Author

@Gordonby If I run the script with "./generate-vm-sku-list-v2.sh" it works and when I run it with "sh generate-vm-sku-list-v2.sh" I get the same error you get. I'll have a look at why

@fireblade95402
Copy link
Contributor Author

Ok, I'm using "bash" shell and not the "sh" shell. Gather differences in language support. Should we be making it "sh" or would "bash" be ok

@Gordonby
Copy link
Collaborator

Gordonby commented Nov 8, 2022

Needed to add an Azure Login step to the build, but now it works fine :)

image

@Gordonby Gordonby enabled auto-merge (squash) November 8, 2022 16:38
Copy link
Collaborator

@Gordonby Gordonby left a comment

Choose a reason for hiding this comment

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

lgtm.

we may want to revisit the script file to look for potential optimisation... But since it's just called once in release rather than by the UI - i think we're good.

@Gordonby Gordonby merged commit c98db36 into main Nov 8, 2022
@Gordonby Gordonby deleted the mwg-vm-skus branch November 8, 2022 16:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Expand the list of VM SKU's in the Helper dropdown
2 participants