Skip to content

long poll timeout constants#10246

Merged
stephanos merged 3 commits into
mainfrom
stephanos/long-poll-const
May 14, 2026
Merged

long poll timeout constants#10246
stephanos merged 3 commits into
mainfrom
stephanos/long-poll-const

Conversation

@stephanos
Copy link
Copy Markdown
Contributor

@stephanos stephanos commented May 13, 2026

What changed?

Introduces shared constants for default long poll timeout/buffer and applies them to SAA and SANO.

Why?

After a long internal technical discussion, 60s was determined as the default for the timeout.

@stephanos stephanos force-pushed the stephanos/long-poll-const branch 2 times, most recently from 412239e to 10c7a30 Compare May 13, 2026 16:42
@stephanos stephanos marked this pull request as ready for review May 13, 2026 16:42
@stephanos stephanos requested review from a team as code owners May 13, 2026 16:42
@stephanos stephanos changed the title DefaultLongPollTimeout constant long poll timeout constants May 13, 2026
@stephanos stephanos force-pushed the stephanos/long-poll-const branch 5 times, most recently from 88127af to b6e53f7 Compare May 13, 2026 16:49
@stephanos stephanos force-pushed the stephanos/long-poll-const branch from b6e53f7 to f4ba233 Compare May 13, 2026 16:50
Copy link
Copy Markdown
Contributor

@chrsmith chrsmith left a comment

Choose a reason for hiding this comment

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

:shipit:

@stephanos stephanos enabled auto-merge (squash) May 13, 2026 19:01
@stephanos stephanos disabled auto-merge May 13, 2026 19:01
@stephanos
Copy link
Copy Markdown
Contributor Author

Reverting the HistoryLongPollExpirationInterval change again as I saw test flakes.

var LongPollTimeout = dynamicconfig.NewNamespaceDurationSetting(
"nexusoperation.longPollTimeout",
20*time.Second,
common.DefaultLongPollTimeout,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Might want to just double check this test and see if it needs to adjusted based on the new timeout

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

uh good catch!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

actually; let me address that separately

@stephanos stephanos merged commit 4be69e5 into main May 14, 2026
47 checks passed
@stephanos stephanos deleted the stephanos/long-poll-const branch May 14, 2026 20:50
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.

4 participants