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

Adding retry policy to be available to the plugins #112

Merged
merged 1 commit into from
Oct 5, 2020

Conversation

catalinii
Copy link
Contributor

@catalinii catalinii commented Aug 11, 2020

@catalinii catalinii force-pushed the add_retry_info branch 4 times, most recently from f68c493 to e52bd01 Compare August 11, 2020 23:20
@codecov-commenter
Copy link

codecov-commenter commented Aug 11, 2020

Codecov Report

Merging #112 into master will decrease coverage by 0.36%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #112      +/-   ##
==========================================
- Coverage   62.11%   61.74%   -0.37%     
==========================================
  Files         104      104              
  Lines        6440     5775     -665     
==========================================
- Hits         4000     3566     -434     
+ Misses       2069     1843     -226     
+ Partials      371      366       -5     
Impacted Files Coverage Δ
...asks/pluginmachinery/flytek8s/k8s_resource_adds.go 95.06% <100.00%> (-0.35%) ⬇️
go/tasks/plugins/array/awsbatch/job_config.go 77.77% <0.00%> (-5.56%) ⬇️
...uginmachinery/ioutils/precomputed_shardselector.go 45.83% <0.00%> (-4.17%) ⬇️
.../tasks/pluginmachinery/catalog/reader_processor.go 12.50% <0.00%> (-4.17%) ⬇️
...o/tasks/pluginmachinery/ioutils/raw_output_path.go 71.42% <0.00%> (-3.58%) ⬇️
...tasks/pluginmachinery/catalog/async_client_impl.go 38.66% <0.00%> (-3.51%) ⬇️
go/tasks/plugins/array/k8s/launcher.go 44.15% <0.00%> (-3.47%) ⬇️
...luginmachinery/ioutils/remote_file_input_reader.go 8.33% <0.00%> (-3.44%) ⬇️
go/tasks/plugins/array/core/phase_enumer.go 10.00% <0.00%> (-3.34%) ⬇️
go/tasks/pluginmachinery/core/phase_enumer.go 10.00% <0.00%> (-3.34%) ⬇️
... and 95 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 210996c...d7e2d19. Read the comment docs.

@@ -11,6 +12,7 @@ import (
type TaskOverrides interface {
GetResources() *v1.ResourceRequirements
GetConfig() *v1.ConfigMap
GetRetryStrategy() *v1alpha1.RetryStrategy
Copy link
Contributor

Choose a reason for hiding this comment

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

Please do not add references to the CRD. These should be references to the core Protobuf objects. This is a cyclic dependency

@kumare3
Copy link
Contributor

kumare3 commented Aug 17, 2020

@catalinii this is incorrect. PluginMachinery only references the flyteIDL (proto) defs. the existence of CRD is unknown at this point

@catalinii catalinii force-pushed the add_retry_info branch 2 times, most recently from b9ef848 to b9108e0 Compare October 2, 2020 00:24
@catalinii catalinii force-pushed the add_retry_info branch 6 times, most recently from 92f28df to a661204 Compare October 5, 2020 23:00
@catalinii catalinii merged commit ab85481 into master Oct 5, 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