Skip to content

fix: use ADK_GCS_BUCKET_NAME in periodic_cleanup - #183

Open
chauhanyash-19 wants to merge 1 commit into
fetchai:mainfrom
chauhanyash-19:fix/periodic-cleanup-hardcoded-bucket
Open

fix: use ADK_GCS_BUCKET_NAME in periodic_cleanup#183
chauhanyash-19 wants to merge 1 commit into
fetchai:mainfrom
chauhanyash-19:fix/periodic-cleanup-hardcoded-bucket

Conversation

@chauhanyash-19

Copy link
Copy Markdown

Summary

periodic_cleanup in the AI due-diligence agent's executor.py always
targeted a hardcoded GCS bucket ("ai-due-diligence-agent") instead of
reading ADK_GCS_BUCKET_NAME, the same env var GcsArtifactService uses.
On deployments with a custom bucket configured, the hourly cleanup job
silently cleaned the wrong bucket while logging deleted/retained counts
that described data it never touched. This PR reads the bucket name the
same way the artifact service does, so the write path and the cleanup
path always agree.

Type of Change

  • New agent example
  • Bug fix
  • Documentation update
  • Refactor / cleanup
  • Other

Checklist

  • I have starred this repository.
  • New community agents are under contributors/<agent-name>/ (not repo root).
  • I ran ruff check ..
  • I ran ruff format ..
  • I added/updated README.md for changed example(s).
  • I added .env.example if environment variables are required.
  • I added demo image/GIF (if applicable).
  • I added agent profile link (if applicable).
  • I updated contributors/CHANGELOG.md, if this PR changes a community agent.
  • I added my agent to the Community Contributors table in root README.md (if new agent).
  • I verified paths/commands used in docs.
  • I understand this PR requires maintainer review before merge (review-required CI).

Related Issue

Fixes #180

Notes for Reviewers

One-line fix: client.bucket("ai-due-diligence-agent")
client.bucket(os.getenv("ADK_GCS_BUCKET_NAME", "ai-due-diligence-agent")),
matching the existing lookup in the GcsArtifactService setup further
down in the same file. Added tests/test_periodic_cleanup.py with two
cases (configured bucket, default fallback) — verified the new test
fails against the old hardcoded code before the fix, and passes after.

@github-actions

Copy link
Copy Markdown

AI code review

This PR fixes periodic_cleanup to use the same ADK_GCS_BUCKET_NAME environment variable that GcsArtifactService uses, ensuring cleanup targets the configured bucket. The change is minimal, well-tested with mocked unit tests, and follows the existing pattern in the codebase. No blocking issues found.

How I read this change
  • executor.py: Changed periodic_cleanup to read ADK_GCS_BUCKET_NAME env var with fallback, matching the GcsArtifactService initialization at line 205.
  • executor.py: Verified os is imported at line 7, storage.Client at line 29, and the env var pattern is consistent across the file.
  • tests/conftest.py: Added test configuration that sets placeholder credentials and adds the package root to sys.path for imports.
  • tests/test_periodic_cleanup.py: Added two async tests that mock storage.Client and verify bucket name is passed correctly when env var is set or unset.
  • tests/test_periodic_cleanup.py: Tests use monkeypatch for env var manipulation and patch the storage.Client import path correctly.

No blocking issues found in this diff.


This check passes. Findings are advisory.

Reviewed by ASI:One (asi1). Automated review is fallible — apply judgement.

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.

Code issue: periodic cleanup targets a hardcoded GCS bucket, so a configured bucket is never cleaned

1 participant