Skip to content

Pipelining followups#8

Closed
tzaffi wants to merge 9 commits into
file-plugins-msgpfrom
pipelining-followups
Closed

Pipelining followups#8
tzaffi wants to merge 9 commits into
file-plugins-msgpfrom
pipelining-followups

Conversation

@tzaffi

@tzaffi tzaffi commented Aug 29, 2023

Copy link
Copy Markdown
Owner

See: algorand#147 for the continuation of this work

Comment on lines -699 to -700

<-p.ctx.Done()

Copy link
Copy Markdown
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
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

^^^^ !!!

Copy link
Copy Markdown

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?

Comment thread conduit/plugins/importers/noop/sample.yaml Outdated
Comment thread pkg/cli/cli_test.go Outdated
Comment thread pkg/cli/cli_test.go
})
}

func TestHealthEndpoint(t *testing.T) {

Copy link
Copy Markdown

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

Copy link
Copy Markdown

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

tzaffi commented Aug 29, 2023

Copy link
Copy Markdown
Owner Author

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