-
-
Notifications
You must be signed in to change notification settings - Fork 3
chore: whoami details #189
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
Conversation
echarles
left a comment
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.
LGTM
❌ Deploy Preview for datalayer-core failed. Why did it fail? →
|
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.
Pull request overview
This PR enhances the whoami command in the Datalayer CLI to support detailed user information display through a new --details flag. The change provides users with more comprehensive information about their authenticated profile when needed.
Key Changes
- Added a
--detailsboolean flag to bothwhoamiandwhoami_rootcommands - Implemented detailed user information display including full name, UIDs, roles, avatar, timestamps, connected IAM providers, and credits customer information
- Updated
whoami_rootwrapper function to pass the newdetailsparameter to the underlyingwhoamifunction
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| first_name = user.get("first_name_t", "") | ||
| last_name = user.get("last_name_t", "") | ||
| if first_name or last_name: | ||
| console.print(f"📝 Name: {first_name} {last_name}".strip()) |
Copilot
AI
Dec 15, 2025
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.
The strip() method is being called on the entire formatted string, which will only remove whitespace from the beginning and end of the entire string. This doesn't handle the case where both first_name and last_name are empty strings, which would result in "📝 Name: " being printed. The strip() should be applied to the concatenated name before the f-string formatting, or the entire print statement should be skipped if the concatenated name is empty after stripping.
|
|
||
| # Avatar | ||
| if user.get("avatar_url_s"): | ||
| console.print(f"🖼️ Avatar: {user.get('avatar_url_s')}") |
Copilot
AI
Dec 15, 2025
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.
There are two spaces after the emoji (🖼️ Avatar) which appears to be inconsistent with other emoji usage in the same function where there is only one space (e.g., "📝 Name:", "🆔 UID:", "🔑 ID:").
| console.print(f"🖼️ Avatar: {user.get('avatar_url_s')}") | |
| console.print(f"🖼️ Avatar: {user.get('avatar_url_s')}") |
| details: bool = typer.Option( | ||
| False, | ||
| "--details", | ||
| help="Show detailed user information", | ||
| ), |
Copilot
AI
Dec 15, 2025
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.
The new --details flag functionality lacks test coverage. The existing test for whoami only checks for "Profile" in the output, but doesn't test the new detailed information display. Consider adding a test case that uses the --details flag to ensure the new functionality works correctly and displays all the detailed fields (name, UID, roles, avatar, timestamps, connected accounts, etc.).
No description provided.