-
Notifications
You must be signed in to change notification settings - Fork 3
Cli UI Patch #192
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
base: main
Are you sure you want to change the base?
Cli UI Patch #192
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Greptile Summary
This PR enhances the user experience in the CLI's buy functionality by addressing potential confusion around zone and instance type precedence. The changes implement two key improvements:
Warning System: When users specify both a zone/cluster and an instance type, the CLI now displays a console warning explaining that the zone takes precedence over the instance type. This prevents user confusion about which parameter will actually be used.
Enhanced Quote Display: The quote display logic has been improved to show more meaningful information when working with zones. Instead of displaying the generic instance type (which would be ignored anyway), the system now fetches zone metadata from the /v0/zones
API endpoint and displays the zone's delivery type and hardware type in a formatted string like "Kubernetes (h100v)" or "Virtual Machine (h100v)".
The implementation adds a new getZoneMetadata
function that queries the zones API to retrieve delivery type and hardware type information. A local DeliveryTypeMetadata
mapping converts API delivery types ("K8s", "VM") to user-friendly display names ("Kubernetes", "Virtual Machine"). This change integrates well with the existing quote display system and maintains consistency with how instance types are presented elsewhere in the codebase.
These changes fit naturally into the existing buy workflow, enhancing the preAction
hook with the warning logic and the quote display section with the improved type information, making the CLI more informative and user-friendly.
Confidence score: 4/5
- This PR is safe to merge with minimal risk of breaking existing functionality
- Score reflects straightforward logic additions with proper error handling and fallbacks
- Pay attention to the API integration in the getZoneMetadata function for potential network-related edge cases
1 file reviewed, no comments
// let user know if they're using a zone or cluster and it's overriding the instance type | ||
if (type && (zone || cluster)) { | ||
console.warn( | ||
`Warning: Zone '${zone}' takes precedence over instance type '${type}'`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic: Warning message uses zone
variable but should use zone || cluster
since cluster could be set without zone
`Warning: Zone '${zone}' takes precedence over instance type '${type}'`, | |
`Warning: Zone '${zone || cluster}' takes precedence over instance type '${type}'`, |
This pr