-
Notifications
You must be signed in to change notification settings - Fork 0
feat(posix): implement POSIX compliance for CLI standards #21
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?
Conversation
… vars Co-authored-by: ayeshurun <98805507+ayeshurun@users.noreply.github.com>
Co-authored-by: ayeshurun <98805507+ayeshurun@users.noreply.github.com>
Co-authored-by: ayeshurun <98805507+ayeshurun@users.noreply.github.com>
Co-authored-by: ayeshurun <98805507+ayeshurun@users.noreply.github.com>
ayeshurun
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.
@copilot please address
docs/POSIX_COMPLIANCE.md
Outdated
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.
@copilot please remove new .md files
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.
Removed all .md documentation files in commit 2cd02b4.
src/fabric_cli/core/fab_constant.py
Outdated
| # Env variables (POSIX compliant: uppercase with underscores) | ||
| FAB_TOKEN = "FAB_TOKEN" | ||
| FAB_TOKEN_ONELAKE = "FAB_TOKEN_ONELAKE" | ||
| FAB_TOKEN_AZURE = "FAB_TOKEN_AZURE" | ||
| FAB_SPN_CLIENT_ID = "FAB_SPN_CLIENT_ID" | ||
| FAB_SPN_CLIENT_SECRET = "FAB_SPN_CLIENT_SECRET" | ||
| FAB_SPN_CERT_PATH = "FAB_SPN_CERT_PATH" | ||
| FAB_SPN_CERT_PASSWORD = "FAB_SPN_CERT_PASSWORD" | ||
| FAB_SPN_FEDERATED_TOKEN = "FAB_SPN_FEDERATED_TOKEN" | ||
| FAB_TENANT_ID = "FAB_TENANT_ID" | ||
|
|
||
| FAB_REFRESH_TOKEN = "FAB_REFRESH_TOKEN" | ||
| IDENTITY_TYPE = "IDENTITY_TYPE" | ||
| FAB_AUTH_MODE = "FAB_AUTH_MODE" # Kept for backward compatibility | ||
| FAB_AUTHORITY = "FAB_AUTHORITY" |
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.
@copilot please keep the current format of values and revert the changes marked.
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.
Reverted environment variable format to lowercase in commit 2cd02b4. Constants now use the original format (e.g., "fab_token", "fab_tenant_id").
docs/POSIX_COMPLIANCE.md
Outdated
| parser.add_argument("-help", action="help") # ❌ Single-dash long option | ||
| ``` | ||
|
|
||
| #### New Implementation (POSIX-Compliant) |
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.
@copilot please scan the definition of commands, under src/fabric_cli/parsers and make sure all the commands are POSIX-compliant.
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.
Scanned all parsers in src/fabric_cli/parsers/ - all are POSIX compliant. All commands correctly use:
- Single dash for single-char options:
-f,-l,-o,-i,-q - Double dash for multi-char options:
--force,--long,--output,--input,--query
No violations found across all 11 parser files.
…X compliance Co-authored-by: ayeshurun <98805507+ayeshurun@users.noreply.github.com>
| ERROR_UNIVERSAL_SECURITY_DISABLED = "UniversalSecurityDisabled" | ||
| ERROR_SPN_AUTH_MISSING = "ServicePrincipalAuthMissing" | ||
| ERROR_JOB_FAILED = "JobFailed" | ||
|
|
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.
@copilot please scan the cli commands and make sure they are complaint to POSIX
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.
Scanned all CLI commands for POSIX compliance. Fixed 3 violations in commit 40f2a03:
- fab_describe_parser.py: Replaced hardcoded
sys.exit(2)withEXIT_CODE_CANCELLED_OR_MISUSE_BUILTINS - fab_parser_setup.py: Replaced hardcoded
sys.exit(2)withEXIT_CODE_CANCELLED_OR_MISUSE_BUILTINS - fab_config_set.py: Replaced
os._exit(0)(private function) withsys.exit(EXIT_CODE_SUCCESS)
All CLI commands now properly use POSIX-compliant exit code constants. Verified: exit codes (0, 1, 2, 126, 127, 128+n), error handling, signal handling, and output streams all compliant.
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.
@copilot i meant also the fs command file system POSIX complaint
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.
Scanned all filesystem (fs) commands for POSIX compliance. Fixed 2 violations in commit b8ba571:
- fab_fs_mv_folder.py: Replaced hardcoded
return 0/1withEXIT_CODE_SUCCESS/EXIT_CODE_ERROR - fab_fs_cp_folder.py: Replaced hardcoded
return 0withEXIT_CODE_SUCCESS
All fs commands now use POSIX-compliant exit code constants. Helper functions returning item IDs (not exit codes) are correct as-is. All fs command parsers already use proper POSIX option syntax.
Co-authored-by: ayeshurun <98805507+ayeshurun@users.noreply.github.com>
…h constants Co-authored-by: ayeshurun <98805507+ayeshurun@users.noreply.github.com>
📥 Pull Request
✨ Description of new changes
The CLI violated POSIX standards in exit codes, signal handling, and help flags. This implements POSIX.1-2017 compliance while maintaining backward compatibility for environment variables.
Changes
Exit Codes
Signal Handling
Help/Version Flags
-help(non-POSIX single-dash long option)-h/--help(POSIX compliant)-Vas alternative to-vfor versionCommand Implementation Compliance
fab_describe_parser.py: Replaced hardcodedsys.exit(2)with exit code constantfab_parser_setup.py: Replaced hardcodedsys.exit(2)with exit code constantfab_config_set.py: Replacedos._exit(0)(private function) with propersys.exit()using exit code constantfab_fs_mv_folder.py: Replaced hardcodedreturn 0/1with exit code constantsfab_fs_cp_folder.py: Replaced hardcodedreturn 0with exit code constantsrc/fabric_cli/parsers/are POSIX compliant-f,-l,-o)--force,--long,--output)Breaking Changes
Exit code 4 replaced with 126:
Testing
Files Modified
src/fabric_cli/core/fab_constant.py- Exit codes (env vars unchanged)src/fabric_cli/core/fab_decorators.py- Exit code referencessrc/fabric_cli/main.py- Signal handlers for graceful shutdownsrc/fabric_cli/parsers/fab_global_params.py- Help flag cleanupsrc/fabric_cli/core/fab_parser_setup.py- Version flag enhancement, exit code constant usagesrc/fabric_cli/parsers/fab_describe_parser.py- Exit code constant usagesrc/fabric_cli/commands/config/fab_config_set.py- Exit code constant usage, replaced privateos._exit()src/fabric_cli/commands/fs/mv/fab_fs_mv_folder.py- Exit code constant usagesrc/fabric_cli/commands/fs/cp/fab_fs_cp_folder.py- Exit code constant usagetests/test_posix_compliance.py- Test suite (new)✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.