JSON Standardization and Simpler Parameter Type Support#251
JSON Standardization and Simpler Parameter Type Support#251mohamedzeidan2021 wants to merge 1 commit intoaws:mainfrom
Conversation
|
I also have changes in the I think your change can be made following my change to expand the support for
Also the generate_click_command function could be shared across inference_utils and training_utils (and maybe init_utils, but may be a stretch) |
mollyheamazon
left a comment
There was a problem hiding this comment.
Needs further examination after this PR is merged: https://github.com/aws/private-sagemaker-hyperpod-cli-staging/pull/241
| - JSON single object: '{"key": "value", "key2": "value2"}' | ||
| - JSON array (if allow_multiple=True): '[{"key": "value"}, {"key2": "value2"}]' | ||
| - Key-value single: 'key=value,key2=value2' | ||
| - Multiple flags (if allow_multiple=True): --param obj1 --param obj2 |
There was a problem hiding this comment.
if we are supporting muliple formats, whats the recommendation via help? Is there room for help choosing a certain pattern for recommendation. Say volume example shows: key=value,key2=value2
Previously, there were 16 different parameter inout formats across commands, creating poor user experience where users had to remember different syntaxes for different parameter types.
'[python, train.py]'vs"cluster1,cluster2"vs"['STATUS1', 'STATUS2']"key=value,key2=value2format supportedSolution
Unified Parsing System
Created a centralized parameter parsing module that standardizes JSON as base format while providing simpler syntax alternatives for common use cases
Simple Parameter Type Syntaxes
Implementation
Tests
TestListParameters (13 tests)
Tests the
parse_list_parameter()function to ensure it correctly handles both JSON array format (["item1", "item2"]) and simple list format ([item1, item2]).TestDictParameters (13 tests)
Tests the
parse_dict_parameter()function to validate parsing of both JSON object format ({"key": "value"}) and simple dictionary format ({key: value}).TestComplexObjectParameters (12 tests)
Tests the
parse_complex_object_parameter()function which supports JSON objects, JSON arrays, key-value strings (key=value,key2=value2), and multiple flag usage.TestCommaSeparatedList (5 tests)
Tests the legacy
parse_comma_separated_list()function to ensure backward compatibility with the old comma-separated format (item1,item2,item3).TestEdgeCases (7 tests)
Tests challenging scenarios including Unicode characters, special symbols, nested quotes, file paths, and whitespace handling across all parameter types.
TestErrorMessageQuality (5 tests)
Tests that error messages are user-friendly and informative, containing parameter names, format examples, and clear guidance for invalid input.
TestBackwardCompatibility (3 tests)
Tests that all existing parameter formats from the old system continue to work unchanged, ensuring zero breaking changes.
Total: 61 tests providing comprehensive validation of the unified parameter parsing system.
PR Approval Steps
For Requester
For Reviewer
For Requestersection to double check each item.