-
Notifications
You must be signed in to change notification settings - Fork 13
Feature/extend iss location metrics #202
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
- Add latlong_avg: average of latitude and longitude - Add latlong_sum: sum of latitude and longitude - Add latlong_diff: difference between latitude and longitude 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
WalkthroughAdjusts Docker build cache-busting placement and value, adds derived ISS location metrics, and updates the Fly deployment script to set and later synchronize ANOMSTACK_BUILD_HASH using GIT_COMMIT_HASH. Comments clarified; control flow in deployment gains early and post-deploy secret updates. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev
participant Script as deploy_fly.sh
participant Fly as Fly.io CLI
participant Docker as Docker Build
Dev->>Script: Run deploy_fly.sh
rect rgba(230,245,255,0.6)
note right of Script: Early secret propagation
Script->>Fly: secrets set ANOMSTACK_BUILD_HASH="$GIT_COMMIT_HASH"
end
Script->>Docker: Build image (--no-cache possibly)
Docker-->>Script: Image built
Script->>Fly: Deploy app
Fly-->>Script: Deployment complete
rect rgba(235,255,235,0.6)
note right of Script: Post-deploy synchronization
Script->>Fly: secrets set ANOMSTACK_BUILD_HASH="$GIT_COMMIT_HASH"
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
📊 Test Coverage ReportCoverage: 55% (yellow) ✅ Coverage maintained or improved!
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (5)
metrics/examples/iss_location/iss_location.py (3)
11-12: Good: explicit float parsing; add minimal validation and resilient request handling.Parsing to float is the right move. However, if the API shape changes or returns non-numeric strings, this will raise ValueError and break the ingest. Consider guarding the request and parse with raise_for_status(), JSON errors, and value coercion fallbacks.
Example improvement (outside the changed lines, for clarity):
import pandas as pd import requests def ingest(): url = "http://api.open-notify.org/iss-now.json" try: r = requests.get(url, timeout=10) r.raise_for_status() res = r.json() ts = pd.to_datetime(int(res["timestamp"]), unit="s", utc=True) pos = res["iss_position"] lat = float(pos["latitude"]) lon = float(pos["longitude"]) except (requests.RequestException, ValueError, KeyError) as e: # Prefer consistent schema even on failure; you can choose to raise instead. raise RuntimeError(f"Failed to fetch/parse ISS position: {e}") from e metrics = [ ["iss_latitude", lat], ["iss_longitude", lon], ["iss_latlong_avg", (lat + lon) / 2], ["iss_latlong_sum", lat + lon], ["iss_latlong_diff", lat - lon], ] df = pd.DataFrame(metrics, columns=["metric_name", "metric_value"]) df["metric_timestamp"] = ts return dfNotes:
- Use utc=True when building timestamps to ensure consistent, TZ-aware data across environments.
- New metric names prefixed with iss_ for namespacing.
15-19: Namespace new metrics for consistency with existing “iss_*” metrics.All existing metrics in this example are prefixed with “iss_”. Recommend the same for the derived metrics to simplify downstream filtering and dashboards.
- ["latlong_avg", (lat + lon) / 2], - ["latlong_sum", lat + lon], - ["latlong_diff", lat - lon], + ["iss_latlong_avg", (lat + lon) / 2], + ["iss_latlong_sum", lat + lon], + ["iss_latlong_diff", lat - lon],
10-10: Ruff W293: strip whitespace on blank lines.Ruff flagged trailing whitespace on blank lines. Quick cleanup below.
- + - +Also applies to: 13-13
scripts/deployment/deploy_fly.sh (1)
270-273: Post-deploy secret sync only happens on --force-rebuild; standard path can drift.You sync ANOMSTACK_BUILD_HASH after deploy only in the force rebuild branch. On standard deploys, if the early secrets block didn’t run (see prior comment), the secret may be stale vs the image’s build-arg value.
Options:
- Add the same sync after the standard deploy branch, or
- Ensure the early secrets set is executed (by moving commit hash capture earlier) and remove the post-deploy sync to avoid a second release.
If you prefer symmetry, add after Line 279:
# Keep secret aligned with deployed image (standard deploy) if [[ -n "$GIT_COMMIT_HASH" ]]; then echo "🔄 Updating ANOMSTACK_BUILD_HASH secret to match deployed version..." fly secrets set ANOMSTACK_BUILD_HASH="$GIT_COMMIT_HASH" -a "$APP_NAME" fidocker/Dockerfile.fly (1)
34-37: Provide a safe default for ANOMSTACK_BUILD_HASH to aid local/manual builds.If the build arg isn’t provided (manual builds, CI misconfig), ENV will be empty. A harmless default improves debuggability.
-ARG ANOMSTACK_BUILD_HASH +ARG ANOMSTACK_BUILD_HASH=unknown ENV ANOMSTACK_BUILD_HASH=${ANOMSTACK_BUILD_HASH}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
docker/Dockerfile.fly(1 hunks)metrics/examples/iss_location/iss_location.py(1 hunks)scripts/deployment/deploy_fly.sh(3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Use Ruff for linting with a maximum line length of 100 characters for Python code
Files:
metrics/examples/iss_location/iss_location.py
🧬 Code graph analysis (2)
scripts/deployment/deploy_fly.sh (1)
scripts/deployment/preview_fly_secrets.sh (1)
preview_env_vars_from_file(24-148)
metrics/examples/iss_location/iss_location.py (1)
tests/test_examples.py (1)
test_iss_location_ingest(149-158)
🪛 Ruff (0.12.2)
metrics/examples/iss_location/iss_location.py
10-10: Blank line contains whitespace
Remove whitespace from blank line
(W293)
13-13: Blank line contains whitespace
Remove whitespace from blank line
(W293)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test
🔇 Additional comments (2)
scripts/deployment/deploy_fly.sh (1)
258-263: Comment clarification looks good.Clearer explanation of the multi-pronged cache-busting strategy; no functional change here.
docker/Dockerfile.fly (1)
22-29: Right placement for CACHEBUST to invalidate code-copy layers.Moving the CACHEBUST echo immediately before COPY of application code is the right call: dependency layers stay cached, while app code can be force-refreshed without rebuilding wheels.
| # Always ensure ANOMSTACK_BUILD_HASH is set correctly | ||
| if [[ -n "$GIT_COMMIT_HASH" ]]; then | ||
| all_secrets+=("ANOMSTACK_BUILD_HASH=$GIT_COMMIT_HASH") | ||
| echo "🏷️ Adding build hash to secrets: $GIT_COMMIT_HASH" | ||
| fi | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: GIT_COMMIT_HASH used before it’s defined, so the early secret is never set.
At this point GIT_COMMIT_HASH hasn’t been initialized (it’s defined at Lines 244–250), so [[ -n "$GIT_COMMIT_HASH" ]] is always false and ANOMSTACK_BUILD_HASH never gets added to all_secrets. This breaks the intended “early emission” of the build hash.
Fix options (pick one):
- Preferred: move the “capture git commit hash” block (Lines 244–250) up above this section, then keep this early secret injection.
- Minimal: remove this early block and set ANOMSTACK_BUILD_HASH after computing the hash (before fly secrets set) to avoid an extra release.
Apply removal of the no-op block now:
-if [[ -n "$GIT_COMMIT_HASH" ]]; then
- all_secrets+=("ANOMSTACK_BUILD_HASH=$GIT_COMMIT_HASH")
- echo "🏷️ Adding build hash to secrets: $GIT_COMMIT_HASH"
-fiThen, immediately after computing GIT_COMMIT_HASH (current Lines 244–250), add:
# Ensure build hash is included in the single secrets operation
if [[ -n "$GIT_COMMIT_HASH" ]]; then
all_secrets+=("ANOMSTACK_BUILD_HASH=$GIT_COMMIT_HASH")
echo "🏷️ Adding build hash to secrets: $GIT_COMMIT_HASH"
fi🤖 Prompt for AI Agents
In scripts/deployment/deploy_fly.sh around lines 216-221, remove the no-op early
check that tests GIT_COMMIT_HASH before it is defined; then immediately after
the block that computes GIT_COMMIT_HASH (around lines 244-250) add a guarded
append to all_secrets that checks the newly computed GIT_COMMIT_HASH and, if
non-empty, adds ANOMSTACK_BUILD_HASH to all_secrets and echoes the added build
hash so the single fly secrets operation includes the build hash.
This pull request focuses on improving deployment reliability and cache busting for Docker builds, as well as enhancing metric ingestion in the ISS location example. The changes ensure that cache busting is handled more consistently during force rebuilds, and that the build hash is always correctly set and tracked in both the container and deployment secrets. Additionally, the ISS location metrics now include extra derived values for better observability.
Deployment and Docker build improvements:
docker/Dockerfile.flyto after Python dependency installation but before application code copy, ensuring fresh code is picked up on force rebuilds and incremented the cache busting argument value. [1] [2]scripts/deployment/deploy_fly.shto always set theANOMSTACK_BUILD_HASHsecret from the current git commit hash for deployment traceability, and added a manual secret update after force rebuilds to guarantee consistency. [1] [2]Metrics ingestion enhancements:
metrics/examples/iss_location/iss_location.py, added new derived metrics (latlong_avg,latlong_sum,latlong_diff) based on ISS latitude and longitude for richer monitoring.Summary by CodeRabbit
New Features
Chores