Repository navigation
ADR-0024: research a real AWS client for cloud-waste (Proposed) - #79
Merged
Merged
Conversation
API mapping for both resource types, checked against the real botocore service models: DescribeAddresses/ReleaseAddress unchanged from ADR-0020; DescribeInstances/StopInstances/GetMetricData for the instance side - GetMetricData, not GetMetricStatistics, since it batches up to 500 metrics per call instead of one call per metric. A real gap found, not just a number to re-check: AWS's own published Trusted Advisor algorithm is a per-day boolean counted across a 14-day lookback (flagged on >=4 days), not the single-average-checked-once shape IdleInstanceAdapter actually implements today. The 10%/5MB numbers were right; the shape of the check around them wasn't. Also likely a naming gap: the network threshold reads as a daily total (CloudWatch Sum), not the average avg_network_bytes implies. Credentials: recommends NOT copying ADR-0003/0013's Stripe-shaped pydantic-settings pattern - boto3's own credential chain already handles profiles/IAM roles/SSO correctly, and re-wrapping it would regress real users away from those. Sandbox-first: DryRun gates the two mutating calls (confirmed present on both, plus on the two Describe calls); GetMetricData has no DryRun at all since it's read-only - AWS gives a stronger double-gate than Stripe's key-prefix split ever could, for free. Left open for a grilling round, not decided here: faithful 4-of-14-days algorithm vs. a documented v1 simplification (the one finding that touches already-shipped code); moto vs. hand-monkeypatching boto3 for tests; and whether the maintainer wants to spend anything running this for real, since even a read is a billable AWS API call unlike Stripe's free test-mode API. Filed as #78 (wayfinder:research). Not ready-for-agent yet. Closes #78
This was referenced Sep 5, 2026
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.
Closes #78.
Research output only - ADR-0024, filed as Proposed. Not
ready-for-agentyet; three genuine open questions need a grilling round first, same shape ADR-0021's research went through before its own build issue.Findings
DescribeAddresses/ReleaseAddress, both confirmed to supportDryRun.DescribeInstances,StopInstances, and CloudWatchGetMetricData(notGetMetricStatistics- it batches up to 500 metrics per call). Real shapes confirmed viabotocore's service models.IdleInstanceAdapter(Build idle-EC2-instance detection (v1.1) for cloud-waste, per ADR-0022 #73) actually implements. The 10%/5MB numbers were right; the shape around them wasn't. Likely a related naming gap too: the network threshold reads as a daily total (Sum), not the averageavg_network_bytesimplies.pydantic-settingspattern.boto3's own credential chain already handles env vars/profiles/IAM roles/SSO correctly; re-wrapping it in a project-specific settings class would regress real users away from those.DryRungates the two mutating calls (confirmed present, plus on both read calls, though reads have no side effect to gate).GetMetricDatahas noDryRunat all (read-only). AWS gives a stronger double-gate than Stripe's key-prefix split - a real client can read a real account for real and still structurally never release/stop anything unless a secondallow_live=Trueis passed.ec2:DescribeAddresses,ec2:ReleaseAddress,ec2:DescribeInstances,ec2:StopInstances,cloudwatch:GetMetricData.motopresented as a real alternative to ADR-0013's hand-monkeypatching pattern - a dedicated, actively-maintained library that simulates AWS at the transport layer, catching a class of bug hand-typed canned responses can't. Left open, not decided - a real dependency-vs-plumbing-coverage trade-off.Open questions for a grilling round
Instance/IdleInstanceCandidate/detector.py).motovs. hand-monkeypatchingboto3.FakeCloudClientstays the only thing any test, demo, or CI job in this repo talks to either way.