Infrastructure updates and cleanup#604
Merged
forstmeier merged 3 commits intomasterfrom Jun 30, 2025
Merged
Conversation
Contributor
|
Caution Review failedThe pull request is closed. ## Walkthrough
This update standardizes environment variable naming across infrastructure and application components, updates Pulumi and Docker credential references, and improves automation scripts for infrastructure deployment and teardown. It also modifies project metadata, updates `.gitignore`, introduces new services and Cloud Scheduler jobs, removes Prometheus and Grafana monitoring infrastructure, and adds a new teardown task in the automation configuration.
## Changes
| Files/Paths | Change Summary |
|--------------------------------------------------------------------------------------------------------------|---------------------------------------------------------------------------------------------------------------------------------|
| `.github/workflows/lifecycle.yaml`, `.mise.toml` | Simplified lifecycle workflow to single-line `mise tasks run` commands; added `infrastructure:down` task with explicit stack. |
| `.gitignore` | Added `infrastructure/Pulumi.production.yaml` to ignore; removed `todos.md` from ignore list. |
| `infrastructure/environment_variables.py`, `infrastructure/__main__.py`, `application/datamanager/compose.yaml`,<br>`application/datamanager/src/datamanager/config.py`, `infrastructure/buckets.py` | Renamed environment variables and secret keys for consistency (e.g., `DATA_BUCKET` → `DATA_BUCKET_NAME`, Alpaca and DuckDB keys). |
| `infrastructure/services.py` | Renamed parameter `envs` to `environment_variables`; added `enable_prometheus` flag; updated Docker Hub credential keys to uppercase. |
| `infrastructure/Pulumi.yaml` | Renamed Pulumi project and updated description capitalization. |
| `infrastructure/__main__.py` | Added new services: `positionmanager`, `predictionengine`, `eventtrigger`; replaced Pub/Sub with Cloud Scheduler jobs; updated exports and environment variables. |
| `application/eventtrigger/` (Dockerfile, pyproject.toml, src/eventtrigger/) | Added new FastAPI-based `eventtrigger` service with `/health` and `/trigger` endpoints handling specific event types. |
| `application/eventtrigger/tests/test_eventtrigger_main.py` | Added unit tests for `eventtrigger` endpoints including success and error scenarios. |
| `application/positionmanager/src/positionmanager/main.py`, `portfolio.py` | Renamed parameter `data` to `historical_data` in portfolio optimizer method and call. |
| `application/predictionengine/compose.yaml`, `pyproject.toml`, `src/predictionengine/dataset.py`, `main.py`, `post_processor.py`, `tests/test_post_processor.py`, `miniature_temporal_fusion_transformer.py` | Replaced external `category_encoders` dependency with internal `OrdinalEncoder` implementation; updated dependencies; simplified prediction logic to send average prices; added new env var `POSITIONMANAGER_BASE_URL`. |
| `infrastructure/monitoring.py`, `infrastructure/project.py` | Removed Prometheus and Grafana Cloud Run services and config; added GCP Monitoring NotificationChannels from secret emails; added new GCP services (Cloud Build, Monitoring, Cloud Scheduler). |
| `infrastructure/prometheus.yaml`, `infrastructure/topics.py` | Removed Prometheus config YAML and Pub/Sub topic resource. |
| `pyproject.toml` | Added `application/eventtrigger` workspace member and test path; added lint ignore for predictionengine main.py. |
| `workflows/fetch_data.py` | Changed default GCP project from `fund-alpha` to `pocketsizefund` for service URL fetching. |
## Sequence Diagram(s)
```mermaid
sequenceDiagram
participant GitHub Actions
participant mise
participant Pulumi
GitHub Actions->>mise: Run "infrastructure:up" or "infrastructure:down"
mise->>Pulumi: Execute pulumi up/down with stack specification
Pulumi-->>mise: Deploy or teardown infrastructure
mise-->>GitHub Actions: Report resultsequenceDiagram
participant Cloud Scheduler
participant EventTrigger Service
participant DataManager Service
participant PredictionEngine Service
participant PositionManager Service
Cloud Scheduler->>EventTrigger Service: POST /trigger {event: fetch_data}
EventTrigger Service->>DataManager Service: POST /equity-bars with date
Cloud Scheduler->>EventTrigger Service: POST /trigger {event: create_positions}
EventTrigger Service->>PredictionEngine Service: POST /create-positions
Cloud Scheduler->>EventTrigger Service: POST /trigger {event: close_positions}
EventTrigger Service->>PositionManager Service: DELETE /positions
Possibly related PRs
Suggested reviewers
Poem
|
Contributor
There was a problem hiding this comment.
Pull Request Overview
This pull request updates environment variable names and configuration settings to enforce a consistent naming convention across the infrastructure and application code. Key changes include updating DockerHub credential keys to uppercase, renaming various environment variables (e.g. ALPACA_API_KEY, DATA_BUCKET_NAME) for clarity, and modifying deployment commands to use explicit stack references.
Reviewed Changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| infrastructure/services.py | Updated DockerHub credentials to use uppercase secret names |
| infrastructure/environment_variables.py | Renamed environment variables for consistency (ALPACA, DUCKDB, DATA_BUCKET) |
| infrastructure/buckets.py | Renamed bucket variable from production_data_bucket to data_bucket |
| infrastructure/main.py | Updated environment variable references in service creation |
| infrastructure/Pulumi.yaml | Changed project name and description for consistency |
| application/datamanager/src/datamanager/config.py | Updated default environment variable key and error message in Bucket model |
| application/datamanager/compose.yaml | Changed environment variable reference to DATA_BUCKET_NAME |
| .mise.toml | Updated pulumi commands to use explicit stack names |
| .github/workflows/lifecycle.yaml | Updated workflow commands to use mise tasks for stack operations |
Comments suppressed due to low confidence (6)
infrastructure/services.py:53
- Ensure that updating DockerHub secret names to uppercase aligns with deployment configurations and documentation.
username=config.require_secret("DOCKERHUB_USERNAME"),
infrastructure/environment_variables.py:22
- The variable name change to ALPACA_API_KEY improves clarity; verify that corresponding service configurations are updated accordingly.
ALPACA_API_KEY = create_environment_variable(
infrastructure/buckets.py:7
- [nitpick] Renaming production_data_bucket to data_bucket enhances clarity; please ensure that all references to this bucket are updated.
data_bucket = storage.Bucket(
application/datamanager/src/datamanager/config.py:16
- Updating the environment variable key from DATA_BUCKET to DATA_BUCKET_NAME ensures consistency across the project; confirm that the expectation in error messages remains accurate.
name: str = Field(default=os.getenv("DATA_BUCKET_NAME", ""))
.mise.toml:93
- The explicit stack flag clarifies deployment targeting; ensure the stack name matches the intended production environment.
uv run pulumi up --yes --stack pocketsizefund/pocketsizefund/production
.github/workflows/lifecycle.yaml:22
- Refactoring the workflow to use 'mise tasks run' improves consistency; double-check that the corresponding task definitions are correctly updated.
command: mise tasks run infrastructure:up
Graphite Automations"Assign author to pull request" took an action on this PR • (06/22/25)1 assignee was added to this PR based on John Forstmeier's automation. |
forstmeier
force-pushed
the
06-21-infrastructure_updates_and_cleanup
branch
from
c97e2bf to
adcf666
Compare
…:pocketsizefund/pocketsizefund into 06-21-infrastructure_updates_and_cleanup
forstmeier
force-pushed
the
06-21-infrastructure_updates_and_cleanup
branch
from
adcf666 to
a93307e
Compare
Collaborator
Author
|
This is a ton of slop but it needs to be merged in both to get some of the services talking to each other (ideally) and to hit the June deadline. Most of this will need to be heavily reworked in the immediate future. |
This was referenced Aug 14, 2025
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
f# Overview
Changes
Pulumi.production.yamlforproductionstack (replacinginfrastructure)Comments
A couple things on this:
pocketizefundproject in GCP due to permissioning and to cleanup namingpocketsizefund-databucket and move that to thepocketsizefundproject due to naming collision (currently using a temporary name to avoid it)masterto test the deploy workflow has not been done yetSummary by CodeRabbit
New Features
Improvements
Bug Fixes
Refactor
Tests
Chores
Revert