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

Create executions in flytectl #39

Merged
merged 10 commits into from
Mar 22, 2021
Merged

Create executions in flytectl #39

merged 10 commits into from
Mar 22, 2021

Conversation

pmahindrakar-oss
Copy link
Contributor

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

TL;DR

Create the executions for given workflow/task in a project and domain.

There are three steps in generating an execution.

  • Generate the execution spec file using the get command.
  • Update the inputs for the execution if needed.
  • Run the execution by passing in the generated yaml file.

The spec file should be generated first and then run the execution using the spec file

flytectl get tasks -d development -p flytectldemo core.advanced.run_merge_sort.merge --version v2 --execFile execution_spec.yaml

The generated file would look similar to this. Also if the file already exist it would prompt you if you need to overwrite.

	 iamRoleURN: ''
	 inputs:
	   sorted_list1:
	   - 0
	   sorted_list2:
	   - 0
	 kubeServiceAcct: ""
	 targetDomain: ""
	 targetProject: ""
	 task: core.advanced.run_merge_sort.merge
	 version: "v2"

The generated file can be modified to change the parameter values and can be passed to command for execution

genExecSpecFile: true
	 iamRoleURN: 'arn:aws:iam::12345678:role/defaultrole'
	 inputs:
	   sorted_list1:
	   - 2
	   - 4
	   - 6
	   sorted_list2:
	   - 1
	   - 3
	   - 5
	 kubeServiceAcct: ""
	 targetDomain: "development"
	 targetProject: "flytesnacks"
	 task: core.advanced.run_merge_sort.merge
	 version: "v2"

Notice the source and target domain/projects can be different.
The root project and domain flags of -p and -d should point to task/launch plans project/domain.

flytectl create execution --execFile execution_spec.yaml -p flytectldemo -d development --targetProject flytesnacks

Type

  • Bug Fix
  • Feature
  • Plugin

Are all requirements met?

Unit tests are pending

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

Complete description

How did you fix the bug, make the feature etc. Link to any design docs etc

Tracking Issue

flyteorg/flyte#395

Follow-up issue

NA

cmd/create/execution.go Outdated Show resolved Hide resolved
cmd/create/execution.go Outdated Show resolved Hide resolved
cmd/create/execution.go Outdated Show resolved Hide resolved
cmd/create/execution.go Outdated Show resolved Hide resolved
cmd/create/execution.go Outdated Show resolved Hide resolved
@kumare3
Copy link
Contributor

kumare3 commented Feb 25, 2021

Few comments before you go to far ahead

@kumare3 kumare3 self-requested a review March 2, 2021 05:19
cmd/create/execution.go Outdated Show resolved Hide resolved
cmd/create/execution.go Outdated Show resolved Hide resolved
cmd/create/execution.go Outdated Show resolved Hide resolved
Task string `json:"task,omitempty" pflag:",task name for which execution needs to be launched.Either an execution can be for task or workflow."`
KubeServiceAcct string `json:"kubeServiceAcct" pflag:",kubernetes service account AuthRole for launching execution."`
IamRoleURN string `json:"iamRoleURN" pflag:",iam role URN AuthRole for launching execution."`
Version string `json:"version" pflag:",version of the launch plan or task to be fetched for execution.If not specified it would use the latest version."`
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Version string `json:"version" pflag:",version of the launch plan or task to be fetched for execution.If not specified it would use the latest version."`
Version string `json:"version" pflag:",[optional]version of the launch plan or task to be fetched for execution.If not specified it would use the latest version."`

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since the file has serialized version field always , We should disallow passing this as an override to avoid any mismatch in the generated file task/lp version against the one passed in. Let me know what you think

cmd/create/execution.go Outdated Show resolved Hide resolved
cmd/create/execution.go Outdated Show resolved Hide resolved
cmd/create/execution.go Outdated Show resolved Hide resolved
@pmahindrakar-oss pmahindrakar-oss force-pushed the feature/create-executions branch from 0325cf2 to eb468e2 Compare March 18, 2021 12:59
Signed-off-by: pmahindrakar-oss <[email protected]>
@pmahindrakar-oss pmahindrakar-oss force-pushed the feature/create-executions branch from eb468e2 to 4936d30 Compare March 19, 2021 04:48
Signed-off-by: pmahindrakar-oss <[email protected]>
@pmahindrakar-oss pmahindrakar-oss force-pushed the feature/create-executions branch from cb5444d to 832c408 Compare March 19, 2021 06:58
@codecov
Copy link

codecov bot commented Mar 19, 2021

Codecov Report

Merging #39 (40281b3) into master (0a2f561) will increase coverage by 11.72%.
The diff coverage is 59.71%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master      #39       +/-   ##
===========================================
+ Coverage   34.05%   45.77%   +11.72%     
===========================================
  Files          25       34        +9     
  Lines         693     1018      +325     
===========================================
+ Hits          236      466      +230     
- Misses        441      482       +41     
- Partials       16       70       +54     
Flag Coverage Δ
unittests 45.77% <59.71%> (+11.72%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
cmd/get/execution.go 93.93% <ø> (ø)
cmd/register/register_util.go 24.64% <ø> (ø)
cmd/get/launchplanconfig_flags.go 29.41% <29.41%> (ø)
cmd/get/taskconfig_flags.go 29.41% <29.41%> (ø)
cmd/create/executionconfig_flags.go 36.84% <36.84%> (ø)
cmd/get/execution_util.go 37.50% <37.50%> (ø)
cmd/create/execution.go 55.55% <55.55%> (ø)
cmd/create/serialization_utils.go 60.00% <60.00%> (ø)
cmd/create/execution_util.go 69.76% <69.76%> (ø)
cmd/get/launch_plan_util.go 70.37% <70.37%> (ø)
... and 17 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0a2f561...40281b3. Read the comment docs.

Signed-off-by: pmahindrakar-oss <[email protected]>
Signed-off-by: pmahindrakar-oss <[email protected]>
@pmahindrakar-oss pmahindrakar-oss marked this pull request as ready for review March 19, 2021 12:31
cmd/get/execution_util.go Outdated Show resolved Hide resolved
cmd/get/execution_util.go Outdated Show resolved Hide resolved
cmd/get/execution_util.go Outdated Show resolved Hide resolved
cmd/get/execution_util.go Outdated Show resolved Hide resolved
cmd/get/execution_util.go Outdated Show resolved Hide resolved
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.

Couple nits, then merge?

Signed-off-by: pmahindrakar-oss <[email protected]>
…d empty or nil inputs

Signed-off-by: pmahindrakar-oss <[email protected]>
@kumare3 kumare3 merged commit 0b5ef91 into master Mar 22, 2021
@kumare3 kumare3 deleted the feature/create-executions branch March 22, 2021 04:43
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