Skip to content

fix(flags): pass the resolved polling interval to the definitions poll timer#222

Merged
turnipdabeets merged 2 commits into
PostHog:mainfrom
flivni:polling-interval-default
Jul 17, 2026
Merged

fix(flags): pass the resolved polling interval to the definitions poll timer#222
turnipdabeets merged 2 commits into
PostHog:mainfrom
flivni:polling-interval-default

Conversation

@flivni

@flivni flivni commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

💡 Motivation and Context

feature_flags_polling_interval is documented as "Defaults to 30" and the poller computes @polling_interval = polling_interval || 30, but the value was not actually used, and the 60-second concurrent-ruby was used instead.

This PR fixes the above.

Behavior change worth reviewing: Halves the current effective default. Alternatively, we could update the documented default to 60s.

💚 How did you test it?

  • New "definitions polling interval" specs (2 examples) assert the TimerTask's effective execution_interval: 30 when the option is unset, and the configured value when provided. Before the fix, the default case reports 60.
  • Full suite: 616 examples, 0 failures. rubocop clean. rake public_api:check passes with the snapshot regenerated for the one new constant.

📝 Checklist

  • I reviewed the submitted code.
  • I added tests to verify the changes.
  • I updated the docs if needed.
  • No breaking change or entry added to the changelog.

If releasing new changes

  • Ran pnpm changeset to generate a changeset file

🤖 Agent context

Autonomy: Agent-written, human-directed and reviewed

This bug was found by Claude Code (Fable 5) in a code review on a different PR. All code was written by Claude. My contribution is limited to the "Motivation and Context" paragraph and this paragraph here. I've read the code and I fully understand it.

Comment thread .changeset/polling-interval-default.md Outdated
@turnipdabeets
turnipdabeets requested review from a team July 17, 2026 14:34
@turnipdabeets

turnipdabeets commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

LGTM but @flivni both PRs need a verified signature to merge. Mind adding those? We'll also need to rebase #221 after this one merges

@flivni
flivni force-pushed the polling-interval-default branch from 21f8541 to 887a636 Compare July 17, 2026 20:39
@flivni

flivni commented Jul 17, 2026

Copy link
Copy Markdown
Contributor Author

@turnipdabeets Done — signatures added on both PRs. Can rebase #221 once this one merges.

@turnipdabeets
turnipdabeets enabled auto-merge (squash) July 17, 2026 22:01
@turnipdabeets
turnipdabeets merged commit 26142f5 into PostHog:main Jul 17, 2026
18 checks passed
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