-
Notifications
You must be signed in to change notification settings - Fork 766
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
argo-workflows runner #1868
argo-workflows runner #1868
Conversation
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.
comments for discussion.
|
||
return command_obj | ||
|
||
def deploy(self, **kwargs) -> "DeployedFlow": |
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.
why both deploy and create? Do we have both CLIs?
|
||
self.cwd = cwd | ||
self.spm = SubprocessManager() | ||
self.top_level_kwargs = kwargs |
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 seems to be one level missing. We can have things like: myflow.py <tl options> argo/sfn/whatever <argo/sfn/whatever options> create <create options>
. I only see and here I think.
return self.deployer.trigger(**kwargs) | ||
|
||
|
||
class Deployer(object): |
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.
I am debating whether we should have a common thing like this or let each CLI define everything individually. Pros and cons on both sides.
self.deployer = deployer | ||
|
||
@property | ||
def production_token(self): |
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.
For example, we are getting rid of this so not sure if having it be common is a good idea.
return getattr(api(**top_level_kwargs), _type)(name=name) | ||
|
||
|
||
class TriggeredRun(object): |
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.
Functionally, is there a difference between an ExecutingRun
and a TriggeredRun
?
Closing in favour of #1910 |
No description provided.