Skip to content
This repository has been archived by the owner on May 31, 2024. It is now read-only.

Added command delete along with subcommand executions for terminating #34

Merged
merged 3 commits into from
Feb 17, 2021

Conversation

pmahindrakar-oss
Copy link
Contributor

@pmahindrakar-oss pmahindrakar-oss commented Feb 17, 2021

TL;DR

1] New command for delete

bin/flytectl --help
Warn: No metricsProvider set for the workqueue
flytectl is CLI tool written in go to interact with flyteadmin service

Usage:
  flytectl [command]

Available Commands:
  config      Runs various config commands, look at the help of this command to get a list of available commands..
  delete      Used for terminating/deleting various flyte resources including tasks/workflows/launchplans/executions/project.
  get         Used for fetching various flyte resources including tasks/workflows/launchplans/executions/project.
  help        Help about any command
  register    Registers tasks/workflows/launchplans from list of generated serialized files.
  update      Used for updating flyte resources eg: project.
  version     Displays version information for the client and server.

2] Available subcommands

bin/flytectl delete --help
Warn: No metricsProvider set for the workqueue

Example Delete executions.
::

 bin/flytectl delete execution kxd1i72850  -d development  -p flytesnacks

Usage:
  flytectl delete [command]

Available Commands:
  execution   Terminate/Delete execution resources.

Flags:
  -h, --help   help for delete

3] Example usage
bin/flytectl delete execution c6a51x2l9e eeam9s8sny p4wv4hwgc4 -d development -p flytesnacks

bin/flytectl get execution  -d development  -p flytesnacks
 ------------ ------------------------------------------------------------------------- ---------- ----------- -------------------------------- ---------------
| NAME (7)   | WORKFLOW NAME                                                           | TYPE     | PHASE     | STARTED                        | ELAPSED TIME  |
 ------------ ------------------------------------------------------------------------- ---------- ----------- -------------------------------- ---------------
| c6a51x2l9e | recipes.core.basic.lp.go_greet                                          | WORKFLOW | ABORTED   | 2021-02-17T08:13:04.680476300Z | 15.540361300s |
 ------------ ------------------------------------------------------------------------- ---------- ----------- -------------------------------- ---------------
| eeam9s8sny | recipes.core.basic.lp.go_greet                                          | WORKFLOW | ABORTED   | 2021-02-17T08:14:04.803084100Z | 42.306385500s |
 ------------ ------------------------------------------------------------------------- ---------- ----------- -------------------------------- ---------------
| p4wv4hwgc4 | recipes.core.basic.lp.go_greet                                          | WORKFLOW | ABORTED   | 2021-02-17T08:14:27.476307400Z | 19.727504400s |
 ------------ ------------------------------------------------------------------------- ---------- ----------- -------------------------------- ---------------

4] Documentation result

Screen Shot 2021-02-17 at 9 44 31 PM

Type

  • Bug Fix
  • Feature
  • Plugin

Are all requirements met?

  • Code completed
  • Smoke tested
  • Unit tests added
  • Code documentation added
  • Any pending items have an associated Issue

Complete description

This is similar to flytekit's terminate execution

Tracking Issue

flyteorg/flyte#398

Follow-up issue

NA

Copy link
Contributor

@kumare3 kumare3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome, couple nits. Continueonerror is optional. +1 after

cmd/delete/execution.go Outdated Show resolved Hide resolved
Name: name,
},
})
if err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this also have continue on Error

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought about this option too

In case of register files where we have this continueOnerror option where user can

cat1 ) give list of files from a folder using wildcard/archive and continue the registration on failures , this option seems very much required since without it the only way he has is to remove files in the folder /archive or have some neat way of filtering the bad files or wait until we have an option to accept the list of files from piped o/p

cat2) Another way he can choose to pass the files is by explicitly passing each files absolute path . In this case , the option to remove is much easier since we cant expect him to pass in a long list of files manually .

The execution failures as of now are similar to cat2) where he specifies the list of execution id's and hence removing the failed and reattempting them is easier.

But definitely it would be good to have this option and will take it up in next MR.

@kumare3 kumare3 merged commit 8f99668 into master Feb 17, 2021
@kumare3 kumare3 deleted the feature/delete-executions branch February 17, 2021 16:24
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants