Skip to content

integration: remove default poll delay and timeouts#48956

Merged
thaJeztah merged 1 commit intomoby:masterfrom
thaJeztah:poll_default_delay
Nov 27, 2024
Merged

integration: remove default poll delay and timeouts#48956
thaJeztah merged 1 commit intomoby:masterfrom
thaJeztah:poll_default_delay

Conversation

@thaJeztah
Copy link
Copy Markdown
Member

The default delay is 100ms and default timeout is 10s, so we can remove cases where we are setting the defaults;

config := defaultConfig()
func defaultConfig() *Settings {
return &Settings{Timeout: 10 * time.Second, Delay: 100 * time.Millisecond}

- A picture of a cute animal (not mandatory but encouraged)

@thaJeztah thaJeztah added status/2-code-review area/testing kind/refactor PR's that refactor, or clean-up code labels Nov 26, 2024
@thaJeztah thaJeztah added this to the 28.0.0 milestone Nov 26, 2024
@thaJeztah thaJeztah self-assigned this Nov 26, 2024
cID := container.Run(ctx, t, apiClient, container.WithCmd("sh", "-c", `mkdir /foo; echo xyzzy > /foo/bar`))

poll.WaitOn(t, container.IsInState(ctx, apiClient, cID, "exited"), poll.WithDelay(100*time.Millisecond), poll.WithTimeout(60*time.Second))
poll.WaitOn(t, container.IsInState(ctx, apiClient, cID, "exited"), poll.WithTimeout(60*time.Second))
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Actually curious why we use exited here, and not container.IsStopped - let me also look what the difference is there. (not for this PR)

Copy link
Copy Markdown
Member

@laurazard laurazard left a comment

Choose a reason for hiding this comment

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

LGTM

@thaJeztah
Copy link
Copy Markdown
Member Author

I'll merge this one after #48940, which is more important and I don't know if it conflicts (and too lazy to check 😂)

@thaJeztah thaJeztah merged commit b8db6d1 into moby:master Nov 27, 2024
@thaJeztah thaJeztah deleted the poll_default_delay branch November 27, 2024 21:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/testing kind/refactor PR's that refactor, or clean-up code status/2-code-review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants