Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pipelining followups #8

Closed
wants to merge 9 commits into from
Closed

Conversation

tzaffi
Copy link
Owner

@tzaffi tzaffi commented Aug 29, 2023

See: algorand#147 for the continuation of this work

Comment on lines -699 to -700

<-p.ctx.Done()
Copy link
Owner Author

Choose a reason for hiding this comment

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

This is the health endpoint bugfix.

Zeph Grunschlag added 2 commits August 28, 2023 22:13
// RetriesNoOutput applies the same logic as Retries, but for functions that return no output.
func RetriesNoOutput[X pluginInput](f func(x X) error, a X, p *pipelineImpl, msg string) (time.Duration, error) {
_, d, err := Retries(func(x X) (empty, error) {
// TODO: probly the following function and its unit test should be axed
Copy link
Owner Author

Choose a reason for hiding this comment

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

^^^^ !!!

Choose a reason for hiding this comment

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

I agree that we probably don't need a no-input function right now -- is this+tests+above interface going to be reverted?

pkg/cli/cli_test.go Outdated Show resolved Hide resolved
@@ -135,3 +140,41 @@ func TestLogFile(t *testing.T) {
require.Contains(t, dataStr, "\nWriting logs to file:")
})
}

func TestHealthEndpoint(t *testing.T) {

Choose a reason for hiding this comment

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

nice test 👍

// RetriesNoOutput applies the same logic as Retries, but for functions that return no output.
func RetriesNoOutput[X pluginInput](f func(x X) error, a X, p *pipelineImpl, msg string) (time.Duration, error) {
_, d, err := Retries(func(x X) (empty, error) {
// TODO: probly the following function and its unit test should be axed

Choose a reason for hiding this comment

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

I agree that we probably don't need a no-input function right now -- is this+tests+above interface going to be reverted?

@tzaffi
Copy link
Owner Author

tzaffi commented Aug 29, 2023

closing this PR and will re-open a new one shortly against upstream/master

@tzaffi tzaffi closed this Aug 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants