Apply OSCM naming convention to infrastructure and environment#747
Apply OSCM naming convention to infrastructure and environment#747forstmeier merged 2 commits intomasterfrom
Conversation
Update all infrastructure resources and code references to use OSCM naming convention for consistency and clarity. Changes: - Application code: Update environment variable names from PSF_* to OSCM_* - Rust datamanager: Update default S3 bucket name to oscm-data - Infrastructure: Rename all AWS resources to use oscm prefix - Environment: Update Flox environment name to "fund" - Project: Update pyproject.toml name to "fund" Infrastructure resources updated: - S3 buckets, ECR repositories, ECS services - Security groups, service discovery namespace - CloudWatch log groups, IAM roles and policies - SSM parameters, load balancer, target groups Environment variable changes: - PSF_DATAMANAGER_BASE_URL → OSCM_DATAMANAGER_BASE_URL - PSF_EQUITYPRICEMODEL_BASE_URL → OSCM_EQUITYPRICEMODEL_BASE_URL - PSF_UNCERTAINTY_THRESHOLD → OSCM_UNCERTAINTY_THRESHOLD Resolves #697 Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR implements a consistent naming convention across the codebase, transitioning from "pocketsizefund" to "oscm" for infrastructure resources and environment variables, while aligning project-level naming with the repository name "fund".
Changes:
- Updated all AWS infrastructure resources to use "oscm" prefix (S3 buckets, ECR repositories, ECS services, security groups, IAM roles, etc.)
- Renamed environment variables from
PSF_*toOSCM_*pattern - Updated project and environment names to "fund" to match repository name
Reviewed changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| pyproject.toml | Changed project name from "pocketsizefund" to "fund" |
| maskfile.md | Updated title from "Pocket Size Fund" to "OSCM" |
| infrastructure/parameters.py | Updated SSM parameter paths and tags from "pocketsizefund" to "oscm" |
| infrastructure/main.py | Renamed all AWS resources and environment variables to use "oscm" prefix |
| infrastructure/Pulumi.yaml | Updated Pulumi project name and description to use "oscm" |
| applications/portfoliomanager/src/portfoliomanager/server.py | Changed environment variable names from PSF_* to OSCM_* |
| applications/portfoliomanager/src/portfoliomanager/risk_management.py | Updated UNCERTAINTY_THRESHOLD environment variable to OSCM_* |
| applications/equitypricemodel/src/equitypricemodel/server.py | Changed DATAMANAGER_BASE_URL environment variable to OSCM_* |
| applications/datamanager/src/state.rs | Updated default S3 bucket name from "pocketsizefund-data" to "oscm-data" |
| .github/workflows/close_stale_issues_and_pull_requests.yaml | Added actions and contents permissions |
| .flox/env.json | Changed environment name from "pocketsizefund" to "fund" |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
📝 WalkthroughWalkthroughThis PR standardizes naming from "pocketsizefund"/"PSF" to "oscm"/"OSCM" across infra and app configs, updates several environment variable keys and SSM parameter names, adjusts type hints/casts for Polars DataFrame usage, and expands workflow permissions. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
Greptile OverviewGreptile SummaryThis PR applies a comprehensive naming convention update from Key Changes:
Issue Found:
Infrastructure Impact:
Confidence Score: 3/5
|
| Filename | Overview |
|---|---|
| applications/equitypricemodel/src/equitypricemodel/server.py | Updated env var PSF_DATAMANAGER_BASE_URL to OSCM_DATAMANAGER_BASE_URL and added Polars type casts |
| applications/portfoliomanager/src/portfoliomanager/server.py | Updated env vars PSF_DATAMANAGER_BASE_URL and PSF_EQUITYPRICEMODEL_BASE_URL to OSCM_* and added type cast |
| infrastructure/main.py | Comprehensive rename: all AWS resources, secrets, env vars, and exports updated from pocketsizefund/PSF_* to oscm/OSCM_* |
| maskfile.md | Updated title to OSCM but missed updating psf_base_url references in invoke command (lines 250, 253) |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
maskfile.md (2)
250-256:⚠️ Potential issue | 🔴 CriticalStale reference to
psf_base_urlwill cause runtime failure.The Pulumi output was renamed from
psf_base_urltooscm_base_urlininfrastructure/__main__.py(line 1150), but this script still references the old name. Theinvokecommand fordatamanagerandportfoliomanagerwill fail because the output no longer exists.🐛 Proposed fix
-base_url=$(pulumi stack output psf_base_url --stack production 2>/dev/null || echo "") +base_url=$(pulumi stack output oscm_base_url --stack production 2>/dev/null || echo "") if [ -z "$base_url" ]; then - echo "psf_base_url not found - infrastructure might not be deployed" + echo "oscm_base_url not found - infrastructure might not be deployed" exit 1 fi
86-86: 🧹 Nitpick | 🔵 TrivialConsider renaming the Docker buildx builder for consistency.
The buildx builder is still named
psf-builder. For consistency with the OSCM naming convention, consider renaming it tooscm-builder.♻️ Proposed fix
-docker buildx create --use --name psf-builder 2>/dev/null || docker buildx use psf-builder || (echo "Using default buildx builder" && docker buildx use default) +docker buildx create --use --name oscm-builder 2>/dev/null || docker buildx use oscm-builder || (echo "Using default buildx builder" && docker buildx use default)
🤖 Fix all issues with AI agents
In `@applications/equitypricemodel/src/equitypricemodel/server.py`:
- Line 61: The environment variable lookup for DATAMANAGER_BASE_URL currently
only checks OSCM_DATAMANAGER_BASE_URL and defaults to "http://datamanager:8080",
which can break setups still using the old PSF_DATAMANAGER_BASE_URL; update the
logic that sets DATAMANAGER_BASE_URL to first try
os.getenv("OSCM_DATAMANAGER_BASE_URL"), then fall back to
os.getenv("PSF_DATAMANAGER_BASE_URL") (or the default) so legacy deployments
continue to work, and ensure the symbol DATAMANAGER_BASE_URL is assigned using
that prioritized lookup.
In `@applications/portfoliomanager/src/portfoliomanager/server.py`:
- Around line 66-71: The environment variable constants DATAMANAGER_BASE_URL and
EQUITYPRICEMODEL_BASE_URL currently only read OSCM_* names, which will break
deployments still using PSF_*; change the getenv logic for DATAMANAGER_BASE_URL
and EQUITYPRICEMODEL_BASE_URL to first check the OSCM_* var, then fall back to
the corresponding PSF_* var (and finally to the existing default URL) so both
new and legacy env names are honored during rollout; update any similar
constants to follow this OSCM then PSF then default pattern and keep names
DATAMANAGER_BASE_URL and EQUITYPRICEMODEL_BASE_URL to avoid downstream changes.
Add explicit type casting to resolve type checker errors where Polars operations return InProcessQuery | DataFrame union types but code expects plain DataFrame objects. Changes: - Add typing.cast imports to affected files - Cast .collect() results to DataFrame type - Cast schema validation results to DataFrame type Files updated: - predictions_schema.py: Cast grouped aggregations and lazy collects - portfolio_schema.py: Cast aggregation results - server.py (both): Cast schema validation results - tide_data.py: Cast data validation result All Python checks now pass: - Formatting: passed - Linting: passed - Type checking: passed (15 errors resolved) - Tests: 79 passed - Coverage: 94% Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
bcc986b
Additional Comments (1)
Prompt To Fix With AIThis is a comment left during a code review.
Path: maskfile.md
Line: 250:253
Comment:
Pulumi output variable name not updated - should be `oscm_base_url` instead of `psf_base_url` (updated in `infrastructure/__main__.py:1150`)
```suggestion
base_url=$(pulumi stack output oscm_base_url --stack production 2>/dev/null || echo "")
if [ -z "$base_url" ]; then
echo "oscm_base_url not found - infrastructure might not be deployed"
```
How can I resolve this? If you propose a fix, please make it concise. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@applications/equitypricemodel/src/equitypricemodel/predictions_schema.py`:
- Line 45: The variable lazy_frame is misleading because after calling
data.lazyframe.collect() it contains a concrete pl.DataFrame; rename the
variable (e.g., to df or data_frame) wherever it is defined and referenced to
reflect its actual type and update the cast expression cast("pl.DataFrame",
data.lazyframe.collect()) to use the new name (references: lazy_frame,
data.lazyframe.collect(), cast, pl.DataFrame) so readers aren’t confused about
LazyFrame vs DataFrame.
Overview
Apply consistent OSCM naming convention across infrastructure resources and application code.
Changes
PSF_*toOSCM_*pocketsizefund-datatooscm-dataoscmprefixfund(matches repository name)fund(matches repository name)Infrastructure resources updated:
oscm-data-*,oscm-model-artifacts-*oscm/*oscm-*oscm-alb,oscm-ecs-tasks,oscm-vpc-endpointsoscm.local/ecs/oscm/*oscm-*/oscm/*oscm-*Environment variable changes:
PSF_DATAMANAGER_BASE_URL→OSCM_DATAMANAGER_BASE_URLPSF_EQUITYPRICEMODEL_BASE_URL→OSCM_EQUITYPRICEMODEL_BASE_URLPSF_UNCERTAINTY_THRESHOLD→OSCM_UNCERTAINTY_THRESHOLDContext
This change implements a cleaner naming convention that separates project-level naming (using "fund" for the repository/environment) from infrastructure-level naming (using "oscm" as a concise organization identifier). This approach:
All comprehensive checks passed:
Summary by CodeRabbit