Skip to content

Conversation

@dominik3141
Copy link
Contributor

@dominik3141 dominik3141 commented Oct 31, 2025

  • Need to have cron job checkout main before marking PR as ready!

Two sets of changes

  • Various improvements to the script that actually updates the PR embedding cache
  • Fix some things in the cron job helm file to make sure the correct script is run and the full git history is actually available to the script. And importantly make sure to use uv run with --no-sync to make sure that we don't install hundreds of large dependencies that we never need.

Asana Task

- Add safeguards to prevent embedding all PRs when cache download fails
- Add --force-rebuild flag for explicit full rebuilds
- Add --allow-full-rebuild flag for first-time setup
- Update cronjob to use cache_refresh.py wrapper
- Merge cache_refresh.py functionality into build_cache.py
- Add --download-from-s3 flag for cronjob use
- Add --s3-bucket and --s3-prefix flags for S3 configuration
- Update cronjob to use unified script with --download-from-s3
- Remove redundant cache_refresh.py wrapper
@dominik3141 dominik3141 changed the title Dominik/fix cronjob Fix cronjob to update PR embedding cache Oct 31, 2025
@dominik3141 dominik3141 changed the title Fix cronjob to update PR embedding cache Fix cron job to update PR embedding cache Oct 31, 2025
@dominik3141 dominik3141 marked this pull request as ready for review October 31, 2025 02:42
raise RuntimeError(f"Failed to upload cache to S3: {e}") from e


def _get_api_key() -> str:
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we use the impl in softmax/?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To get the API key? It's just three lines of code, I don't see the point in wrapping the simple call to boto3 (assuming I understand you correctly).

git remote set-url origin https://github.com/Metta-AI/metta.git
git fetch --unshallow origin main 2>/dev/null || git fetch origin main
git checkout -f origin/main
uv pip install numpy google-genai boto3
Copy link
Contributor

Choose a reason for hiding this comment

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

This works but we can also add these as a dependency group inside project.toml and install through uv

Copy link
Contributor

@nishu-builder nishu-builder left a comment

Choose a reason for hiding this comment

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

Any reason not to put all this in the softmax directory and then use the softmax directory as your working when you do UV sync? That should avoid a lot of UV re-downloading packages hell.

@dominik3141
Copy link
Contributor Author

Any reason not to put all this in the softmax directory and then use the softmax directory as your working when you do UV sync? That should avoid a lot of UV re-downloading packages hell.

I have solved that problem by the uv --no-sync, but I agree that there should be a better way to do that

@dominik3141 dominik3141 enabled auto-merge November 3, 2025 17:40
@dominik3141 dominik3141 added this pull request to the merge queue Nov 3, 2025
Merged via the queue into main with commit e34ccc4 Nov 3, 2025
9 checks passed
@dominik3141 dominik3141 deleted the dominik/fix-cronjob branch November 3, 2025 17:56
rjwalters added a commit that referenced this pull request Nov 3, 2025
Resolved conflicts by:
- Keeping our separate helm chart structure (dashboard-cronjob and pr-similarity-cache-cronjob)
- Incorporating PR #3493 changes to pr-similarity-cache-cronjob command (bash script with git checkout, uv run --no-sync, and --download-from-s3 flag)
- Removing old softmax-cronjob anchor system that was deleted in our branch
- Updated helmfile.yaml to use new chart-based approach instead of YAML anchors

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
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.

3 participants