Repository navigation
Preserve native catalog currencies - #90
VasiliyRad wants to merge 3 commits into
Conversation
dhyabi2
left a comment
There was a problem hiding this comment.
Thanks for asking. I read the source changes in billing.py, catalog.py and loader.py and the docs; I did not run the suite. The shape is what #65 asked for: the amount keeps its currency, and nothing non-USD lands in a field named *_usd. Four things I would look at before merging, in order of how much they could surprise an existing caller.
1. cost_status == "priced" no longer implies cost_usd is not None. In resolve_billing the catalog branch changed from catalog_result.cost_usd is not None to catalog_result.cost is not None. For a non-USD row that yields cost_status="priced", cost_provenance="catalog" and cost_usd=None. Any caller that today does if d.cost_status == "priced": total += d.cost_usd was correct before this PR and breaks after the first non-USD row is added to a catalog. The bundled catalog has none, so nothing breaks today, but it is the one compatibility change in the PR and it is silent. Either a distinct status for "priced, not in USD", or one sentence in the BillingDecision docstring and the changelog saying the implication no longer holds, would cover it.
2. CostResult accepts contradictory states. __post_init__ mirrors cost_usd into cost/currency only when both are unset. CostResult(..., cost_usd=Decimal("1"), currency="EUR") is accepted as written: cost stays None while cost_usd holds a number labelled EUR. Since the point of the PR is that a native amount can never be read as dollars, I would reject cost_usd is not None and currency not in (None, "USD"), and cost_usd != cost when both are set.
3. Gateway USD beside a non-USD catalog row. The comment says "the two are never compared", but the discrepancy computation is above the hunk and not in the diff. If it still reads catalog_result.cost_usd, it is safe by accident (it is None); if it is ever switched to catalog_result.cost, it compares dollars with another currency. A test with a reported USD cost and a EUR catalog row asserting cost_discrepancy_status is None would pin that.
4. The docs say ISO 4217; the loader checks three letters. _CURRENCY_CODE accepts any three letters, so ZZZ loads. That may be intended (it keeps the loader free of a currency table), in which case I would say "a three-letter code" in docs/prices.md rather than "ISO 4217". Related, and not a blocker: _COST_QUANTUM is eight decimal places, which suits every fiat currency but rounds to zero for a unit with more precision than that.
Smaller: the document-level currency must still be USD, so a catalog that is wholly in another currency remains impossible and every row must say so itself. The docs do state this; I mention it only because it is the first thing someone with a non-USD catalog will try.
|
Addressed in 7e9122a:
The document-level USD default remains intentional for this additive change; rows may override it individually. Full verification: |
Addresses #65.
Scope
Adds native-currency support to
metergraph-core: a price row may declare a three-letter currency code, and core carries the computed native amount and currency through catalog resolution and billing. Non-USD amounts never populate fields named*_usd, and no currency conversion is performed.This PR does not add end-to-end non-USD cost tracking to the MeterGraph server or dashboard. Server persistence, mixed-currency aggregation, API representation, and UI display remain out of scope. The bundled catalog remains USD-only; native-currency behavior is available to core consumers and custom catalogs.
Compatibility
Existing USD behavior and legacy
CostResultconstruction remain supported.resolve_billing()also accepts the legacy result shape that exposescost_usdwithout the newcostandcurrencyattributes, treating that representation as USD.cost_status == "priced"now means a native-currency amount is available; callers that specifically require dollars must checkcost_usd.Validation
env PYTHONPATH=core/src:server/src pytest core/tests server/tests -q(862 passed, 12 skipped)