Skip to content
This repository has been archived by the owner on Oct 9, 2023. It is now read-only.

Support plugin override matchable resource #127

Merged
merged 6 commits into from
Sep 24, 2020
Merged

Conversation

katrogan
Copy link
Contributor

@katrogan katrogan commented Sep 22, 2020

TL;DR

This change introduces adding plugin overrides to the flyte workflow CRD when they exist for a matching CreateExecutionRequest.

Additionally, this change also adds a logical merge functionality for plugin overrides so that individual users do not have to do a get before update. Rather, any task type plugin overrides they specify will be saved in addition to existing overrides for other task types. The admin-internal sequence for get & update is not transactional and this could cause small errors to arise, but since system administrators will be infrequently calling the master update endpoint this should not really be a concern.

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

https://docs.google.com/document/d/1OWYJ2ggr0pWvOdOKkf04NLtj2Rf32dMVYvAYMoyeU48/edit?usp=sharing

Tracking Issue

flyteorg/flyte#499

Follow-up issue

NA

@codecov-commenter
Copy link

codecov-commenter commented Sep 22, 2020

Codecov Report

Merging #127 into master will increase coverage by 0.33%.
The diff coverage is 78.98%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #127      +/-   ##
==========================================
+ Coverage   61.27%   61.60%   +0.33%     
==========================================
  Files         105      105              
  Lines        7267     7405     +138     
==========================================
+ Hits         4453     4562     +109     
- Misses       2247     2264      +17     
- Partials      567      579      +12     
Flag Coverage Δ
#unittests 61.60% <78.98%> (+0.33%) ⬆️

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

Impacted Files Coverage Δ
...kg/manager/impl/validation/attributes_validator.go 89.79% <50.00%> (-1.70%) ⬇️
pkg/manager/impl/resources/resource_manager.go 63.92% <73.07%> (+4.49%) ⬆️
pkg/repositories/transformers/resource.go 70.08% <78.57%> (+7.79%) ⬆️
pkg/manager/impl/execution_manager.go 69.05% <89.47%> (+0.48%) ⬆️
pkg/workflowengine/impl/propeller_executor.go 53.12% <100.00%> (+2.79%) ⬆️

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 8aac329...519c644. Read the comment docs.

@katrogan
Copy link
Contributor Author

friendly ping @anandswaminathan @EngHabu

@katrogan katrogan merged commit c5425dd into master Sep 24, 2020
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.

3 participants