Skip to content

feat(async-publisher): degrade AsyncPubSubPublisher to noop when GCP credentials are absent - #411

Merged
julietshen merged 4 commits into
julietshen/noop-publisher-fallbackfrom
julietshen/async-noop-publisher
Jul 10, 2026
Merged

feat(async-publisher): degrade AsyncPubSubPublisher to noop when GCP credentials are absent#411
julietshen merged 4 commits into
julietshen/noop-publisher-fallbackfrom
julietshen/async-noop-publisher

Conversation

@julietshen

Copy link
Copy Markdown
Member

Second piece of #410 (part of #343). Stacked on #388 — base this on julietshen/noop-publisher-fallback; review after #388 lands.

What this changes

Applies the noop-when-GCP-credentials-absent behavior from #388 to the async worker's AsyncPubSubPublisher, and dedups the credential check so both publishers share one implementation.

  • New osprey/worker/lib/gcp_credentials.pygcp_credentials_available(), the cached, lock-guarded probe of google.auth.default() (at most one call per process). Lifted out of the sync PubSubPublisher so there is a single source of truth. A future change (e.g. bounding the metadata probe with a timeout) now lands in one place.
  • Sync publisher.py — drops its private _check_gcp_credentials copy and imports the shared helper. Credential-check tests move to test_gcp_credentials.py; the publisher tests now patch the shared helper directly.
  • Async AsyncPubSubPublisher — detects missing credentials at construction, skips building the PublisherClient, logs a one-time warning naming the inert project and topic, and short-circuits publish_bytes() (both publish entry points route through it) and stop(). Emits async_pubsub_publisher.publish.noop per attempt, tagged with project and topic, for parity with the sync side.

Verification

  • ruff clean across all changed files; mypy clean on both publishers and the shared helper.
  • Async publisher tests: 5 passed (2 existing pinned to the enabled path, 3 new: noop construction, publish noop metric, stop short-circuit).
  • Sync lib/tests are blocked locally by the postgres autouse fixture (same as feat(publisher): degrade PubSubPublisher to noop when GCP credentials are absent #388); verified the sync refactor standalone (shared-helper false/cached paths, sync noop publish/stop, enabled path). CI runs the full suite.

Not in this PR

Broader GCP-eager modules (secret_manager.py, bigquery.py, access_audit_log.py) remain tracked under #343.

🤖 Generated with Claude Code

https://claude.ai/code/session_01TEcfpqhza3dPepZUVWw36X

@coderabbitai

coderabbitai Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

🗂️ Base branches to auto review (1)
  • main

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 7bb4117c-3316-4cd4-bd5e-1dbcb0ae32c2

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch julietshen/async-noop-publisher

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@julietshen
julietshen force-pushed the julietshen/async-noop-publisher branch 2 times, most recently from 4fc4ba4 to 0bdb429 Compare July 9, 2026 20:49
@ThisIsMissEm

Copy link
Copy Markdown
Contributor

I'd be inclined to roll this into #388 instead of keeping it as separate. The changes here lift the detection of GCP credentials up a layer, which is probably the right way forwards. You could then have a metric which is "configuration.errors" which would probably more clearly communicate this issue than at publish time, since it's only checked on startup, and credentials being revoked as the worker is running won't drop into a disabled state, they'll just show up as a failure spike

julietshen and others added 4 commits July 10, 2026 09:23
…CP creds

Extract the cached, lock-guarded GCP credential check out of the sync
PubSubPublisher into a shared osprey.worker.lib.gcp_credentials helper, and
apply the same noop-when-credentials-absent behavior to the async worker's
AsyncPubSubPublisher: skip building the client, log a one-time warning, and
short-circuit the publish and stop paths with an
async_pubsub_publisher.publish.noop metric for dashboard visibility.

Both publishers now share one credential-detection implementation, so a future
change (e.g. bounding the metadata probe with a timeout) lands in one place.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TEcfpqhza3dPepZUVWw36X
The shared credential check moved to gcp_credentials_available; update the
validation-exporter conftest patch to match.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TEcfpqhza3dPepZUVWw36X
Mirror the sync-publisher review changes on AsyncPubSubPublisher: drop the
per-send async_pubsub_publisher.publish.noop metric in favor of the one-time
startup warning, and honor the shared DISABLE_GCP_PUBSUB env opt-out that
short-circuits before the credential probe.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TEcfpqhza3dPepZUVWw36X
@julietshen
julietshen force-pushed the julietshen/async-noop-publisher branch from 0bdb429 to 48cefda Compare July 10, 2026 13:27
@julietshen
julietshen merged commit 48cefda into julietshen/noop-publisher-fallback Jul 10, 2026
2 checks passed
@julietshen
julietshen deleted the julietshen/async-noop-publisher branch July 10, 2026 13:31
@julietshen

Copy link
Copy Markdown
Member Author

(combining this with #388)

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