feat(sdk): rename CLI methods to 'create' #7607
Conversation
|
/test all |
|
Skipping CI for Draft Pull Request. |
|
/hold -- waiting to confirm this alias is desired |
|
/retest |
connor-mccarthy
force-pushed
the
use-create-for-resource-put-in-cli
branch
from
c8e1dbc to
ed62d58
Compare
| import click | ||
|
|
||
|
|
||
| def deprecated_alias_group_factory(deprecated_map: Dict[str, str]): |
|
|
||
|
|
||
| Returns: | ||
| DeprecatedAliasGroup: A class that implements the deprecated alias group. |
There was a problem hiding this comment.
nit: type annotate this in the function signature?
There was a problem hiding this comment.
I'm not sure if this is possible. My attempts:
- This doesn't work, because
DeprecatedAliasGroupis not defined yet:
def deprecated_alias_group_factory(...) -> DeprecatedAliasGroup:- And this doesn't work, because mypy does not support this type for forward ref (mypy error: Name "DeprecatedAliasGroup" is not defined):
def deprecated_alias_group_factory(...) -> 'DeprecatedAliasGroup':I opted for omission over using Any. WDYT?
There was a problem hiding this comment.
def deprecated_alias_group_factory(...) -> 'DeprecatedAliasGroup':
We have been using this forward declaration pattern in our code base. Now I do recall that mypy would complain on such annotation, but we weren't enforcing the mypy check in the past.
One way to make it "work" is to disable the mypy error on this specific line. Then the annotation might be slightly more helpful for devs looking at this code.
It's very minor. Feel free to ignore my original comment. :)
There was a problem hiding this comment.
I like that approach 👍🏻
There was a problem hiding this comment.
Updated (and also fixed one other mypy error)
|
/unhold |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: chensun The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
* add deprecated alias group * add deprecated alias group tests * implement deprecated alias group * clean up other alias tests * clean up docstring * fix type annotations / mypy * format with yapf
* add deprecated alias group * add deprecated alias group tests * implement deprecated alias group * clean up other alias tests * clean up docstring * fix type annotations / mypy * format with yapf
Description of your changes:
Aliases some CLI verbs to be more consistent.
To test on the
kfpCLI:kfp run submit --helpshould work, but raise a warningkfp run create --helpshould work, but not raise a warningkfp pipeline upload --helpshould work, but raise a warningkfp pipeline create --helpshould work, but not raise a warningChecklist:
about the pull request title convention used in this repository.