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

Conversation

pmahindrakar-oss
Copy link
Contributor

@pmahindrakar-oss pmahindrakar-oss commented May 13, 2021

Signed-off-by: Prafulla Mahindrakar [email protected]

TL;DR

This PR specifically addresses supporting plugin overrides through flytectl.

  • GET

Retrieves plugin override for given project,domain combination or additionally with workflow name.

Here the command get plugin override for project flytectldemo and development domain.

flytectl get plugin-override -p flytectldemo -d development

eg : O/P

 {
	"project": "flytectldemo",
	"domain": "development",
	"overrides": [{
		"task_type": "python_task",
		"plugin_id": ["pluginoverride1", "pluginoverride2"],
        "missing_plugin_behavior": 0 
	}]
 }

Writing the plugin override to a file. If there are no plugin overrides, command would return an error
Here the command gets plugin overrides and writes the config file to po.yaml
eg: content of po.yaml

flytectl get plugin-override --attrFile po.yaml

.. code-block:: yaml
    domain: development
    project: flytectldemo
    overrides:
       - task_type: python_task
         plugin_id:
           - plugin_override1
           - plugin_override2
         missing_plugin_behavior: 1 # 0 : FAIL , 1: DEFAULT
  • UPDATE
    Updates plugin override for given project and domain combination or additionally with workflow name.

Updating the plugin override is only available from config file
Here the command updates takes the input for plugin override from the config file po.yaml

eg: content of po.yaml
flytectl update plugin-override -attrFile po.yaml

  • DELETE
    Deletes plugin override for given project and domain combination or additionally with workflow name.
    Deletes plugin override for project and domain
    Here the command delete plugin overrides for project flytectldemo and development domain.

flytectl delete plugin-override -p flytectldemo -d development

Deleting plugin override using config file
Here the command deletes plugin override from the config file po.yaml
eg: content of po.yaml which will use the project domain and workflow name for deleting the resource

flytectl delete plugin-override --attrFile po.yaml

Deleting plugin override for a workflow
Here the command deletes plugin override for a workflow

flytectl delete plugin-override -p flytectldemo -d development core.control_flow.run_merge_sort.merge_sort

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

Tracking Issue

flyteorg/flyte#992

Follow-up issue

NA

@codecov
Copy link

codecov bot commented May 13, 2021

Codecov Report

Merging #69 (3ae026d) into pmahindrakar/matchable-execution-label (be3a158) will increase coverage by 0.29%.
The diff coverage is 63.80%.

❗ Current head 3ae026d differs from pull request most recent head a3634ce. Consider uploading reports for the commit a3634ce to get more accurate results
Impacted file tree graph

@@                            Coverage Diff                             @@
##           pmahindrakar/matchable-execution-label      #69      +/-   ##
==========================================================================
+ Coverage                                   58.64%   58.93%   +0.29%     
==========================================================================
  Files                                          82       89       +7     
  Lines                                        1758     1863     +105     
==========================================================================
+ Hits                                         1031     1098      +67     
- Misses                                        670      707      +37     
- Partials                                       57       58       +1     
Flag Coverage Δ
unittests ?

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

Impacted Files Coverage Δ
cmd/delete/matchable_cluster_resource_attribute.go 100.00% <ø> (ø)
cmd/delete/matchable_task_resource_attribute.go 100.00% <ø> (ø)
cmd/get/matchable_cluster_resource_attribute.go 100.00% <ø> (ø)
cmd/get/matchable_task_resource_attribute.go 100.00% <ø> (ø)
cmd/update/matchable_cluster_resource_attribute.go 100.00% <ø> (ø)
cmd/update/matchable_task_resource_attribute.go 100.00% <ø> (ø)
...nd/executionclusterlabel/attrdeleteconfig_flags.go 20.00% <20.00%> (ø)
...and/executionclusterlabel/attrfetchconfig_flags.go 20.00% <20.00%> (ø)
...nd/executionclusterlabel/attrupdateconfig_flags.go 20.00% <20.00%> (ø)
.../executionqueueattribute/attrdeleteconfig_flags.go 20.00% <20.00%> (ø)
... and 36 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 be3a158...a3634ce. Read the comment docs.

aliases := [][]string{{"cluster-resource-attributes"}, {"executions"}, {"execution-cluster-labels"}, {"execution-queue-attributes"}, {"task-resource-attributes"}}
shortArray := []string{clusterResourceAttributesShort, execCmdShort, executionClusterLabelShort, executionQueueAttributesShort, taskResourceAttributesShort}
longArray := []string{clusterResourceAttributesLong, execCmdLong, executionClusterLabelLong, executionQueueAttributesLong, taskResourceAttributesLong}
useArray := []string{"cluster-resource-attribute", "execution", "execution-cluster-label", "execution-queue-attribute", "plugin-override", "task-resource-attribute"}
Copy link
Contributor

Choose a reason for hiding this comment

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

why do these change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

These are required for the tests whenever new noun is added as a command. It just adding a new noun to the array which got supported in this PR.

"overrides": [{
"task_type": "python_task",
"plugin_id": ["pluginoverride1", "pluginoverride2"],
"missing_plugin_behavior": 0
Copy link
Contributor

Choose a reason for hiding this comment

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

mind commenting on what the enum values represent here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

Copy link
Contributor Author

@pmahindrakar-oss pmahindrakar-oss May 14, 2021

Choose a reason for hiding this comment

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

Have added this in yaml . since this is json o/p and comments are not supported for JSON format.

If i add comments sphinx doesn't like it . I tried both # and //
flytectl_get_plugin-override.rst:45: WARNING: Could not lex literal_block as "json". Highlighting skipped.

project: flytectldemo
overrides:
- task_type: python_task
plugin_id:
Copy link
Contributor

Choose a reason for hiding this comment

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

can you add an explanation of how this override behavior affects plugin execution?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@pmahindrakar-oss pmahindrakar-oss force-pushed the pmahindrakar/matchable-execution-label branch 2 times, most recently from 707ce66 to be3a158 Compare May 14, 2021 08:32
@pmahindrakar-oss pmahindrakar-oss force-pushed the pmahindrakar/matchable-plugin-override branch from 0ea9d15 to 450f562 Compare May 14, 2021 08:34
Signed-off-by: Prafulla Mahindrakar <[email protected]>
@pmahindrakar-oss pmahindrakar-oss force-pushed the pmahindrakar/matchable-plugin-override branch from 450f562 to a3634ce Compare May 14, 2021 08:51
@pmahindrakar-oss
Copy link
Contributor Author

Merging in the base PR 68

@pmahindrakar-oss pmahindrakar-oss merged commit 038d3b5 into pmahindrakar/matchable-execution-label May 14, 2021
@pmahindrakar-oss pmahindrakar-oss deleted the pmahindrakar/matchable-plugin-override branch May 14, 2021 14:10
pmahindrakar-oss added a commit that referenced this pull request May 14, 2021
Signed-off-by: Prafulla Mahindrakar <[email protected]>
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