chore: add lore help and experimental disclaimer #8563
Conversation
✅ Deploy Preview for determined-ui canceled.
|
| @@ -30,6 +30,10 @@ def validate(s: str) -> str: | |||
| return validate | |||
|
|
|||
|
|
|||
| def is_full_git_commit_hash(s: str) -> bool: | |||
| return bool(re.fullmatch(r"[0-9a-f]{40}", s)) | |||
There was a problem hiding this comment.
hashes can have capital letters too but let's go with the smaller subset?
There was a problem hiding this comment.
To be safe, should we s.lower() before testing against the RE?
There was a problem hiding this comment.
If I'm understanding what you're saying correctly I don't think that'd make it safer. my point here was to avoid assuming 40 letter strings that have capital letters as hashes lowering the input defeats that. if we want to make this additional assumption I'd just update the regex to include capital A-F
There was a problem hiding this comment.
That is a good point.
I am not aware that git hash can be upper case as it is in hexadecimal scheme. I am a little confused when would "hashes can have capital letters too" happen?
There was a problem hiding this comment.
I remember reading somewhere that's still valid for git since it's valid hex 🤷🏼
|
Also wonder if it is possible to rename these options to genai in this PR? If not, that is totally ok and I can put out another one to do so. Thank you |
|
thanks for the review.
I'd prefer to leave these separate to avoid adding to the scope. |
There was a problem hiding this comment.
Discussed offline with @hamidzr wrt the comments, approving the change.
fixes an issue that was introduced in #8563 wrt argument parsing when deploying clusters of specific types
Description
Test Plan
deployed successfully while giving a full valid lore hash
Commentary (optional)
https://hpe-aiatscale.atlassian.net/browse/MLG-1453
https://hpe-aiatscale.atlassian.net/browse/MLG-1416
Checklist
docs/release-notes/.See Release Note for details.
Ticket