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

Add GetMaxAttempts() to taskExecutionMetadata #194

Merged
merged 1 commit into from
Oct 9, 2020
Merged

Add GetMaxAttempts() to taskExecutionMetadata #194

merged 1 commit into from
Oct 9, 2020

Conversation

catalinii
Copy link
Contributor

@catalinii catalinii commented Oct 8, 2020

@@ -140,13 +145,18 @@ func (t *Handler) newTaskExecutionContext(ctx context.Context, nCtx handler.Node
}

resourceNamespacePrefix := pluginCore.ResourceNamespace(t.resourceManager.GetID()).CreateSubNamespace(pluginCore.ResourceNamespace(pluginID))
maxAttempts := uint32(0)
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this is valid. Attempt has to be greater than 0 right?

@katrogan @kumare3 Do we set default maxAttempts to 3 ?

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah it looks like we always set the min attempts to 1 + the user specified value: https://github.com/lyft/flytepropeller/blob/96c9ee9e2a9288101de846be8f49fce8c244ea2c/pkg/compiler/transformers/k8s/utils.go#L24

@codecov-io
Copy link

Codecov Report

Merging #194 into master will increase coverage by 0.01%.
The diff coverage is 80.00%.

@anandswaminathan
Copy link
Contributor

+1. (one comment)

@catalinii catalinii merged commit d17ec93 into master Oct 9, 2020
eapolinario pushed a commit to eapolinario/flytepropeller that referenced this pull request Aug 9, 2023
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.

4 participants