config: resolve the legacy timezone names IB states its times in (ibx#335) - #336
config: resolve the legacy timezone names IB states its times in (ibx#335)#336userFRM wants to merge 1 commit into
Conversation
f3b0d3e to
761b41b
Compare
|
Extended the test to cover all seven mappings rather than five.
The date in the test does work here — June separates Worth noting for anyone reading the diff: this guard is environment-dependent by nature. On a host that has |
…#335) `parse_ib_expiry` resolved the zone straight against the system database. `US/Eastern` and its siblings are backward-compatibility links in the tz database, and Debian and Ubuntu ship those in a separate `tzdata-legacy` package that is not installed by default — plain `tzdata` carries no `zoneinfo/US/*` entries at all. On a stock install, including a default container image, the lookup fails. That is IB's own spelling. It is what the gateway sends as a contract's `time_zone_id`, and it is the example the warning a few lines above suggests when a time arrives without a zone. The error never reaches the caller. `attrs()` catches it, logs, and drops the field, so a GTD order whose expiry is stated the way IB states it goes to the gateway with no expiry at all. The order does not expire when it was meant to; it rests, and the only trace is a line in the log. The names are fixed aliases — `US/Eastern` is `America/New_York` and always has been — so resolving them before the lookup makes the parse work wherever the primary names exist, which is everywhere. That is better than requiring a non-default package at deploy time to accept the timezone the vendor uses. This is what the two `config::expiry_tests` failures were. The suite is green on a host without the legacy package now. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Two corrections from the second review, and the first one is the interesting one. The test could not tell a correct target from a wrong one. It compared a single summer instant, so The mapping no longer overrides the host. It rewrote the name before ever asking the system, so a host that does carry the legacy names — or one with a deliberately customised database — had its own answer replaced. Proven with a divergent Independent confirmation worth recording: every pair was checked against |
761b41b to
595a28e
Compare
Summary
parse_ib_expiryresolves the timezone straight against the system database.US/Easternand its siblings are backward-compatibility links, and Debian and Ubuntu ship those in a separatetzdata-legacypackage that is not installed by default.time_zone_id, and the example the warning a few lines above suggests when a time arrives without a zone.Closes #335.
Why it is silent
The parse error never reaches the caller.
attrs()catches it and drops the field:So on any host without the legacy package, a GTD order whose expiry is stated the way IB states it goes out with no expiry. It does not expire when it was meant to — it rests, and the only trace is a line in the log.
Change
The names are fixed aliases:
US/EasternisAmerica/New_Yorkand always has been. Resolving them before the lookup makes the parse work wherever the primary names exist, which is everywhere — better than requiring a non-default package at deploy time to accept the vendor's own timezone.This is what the two failing tests were
config::expiry_tests::named_zone_converts_with_dstandinstant_round_trips_to_wirehave been failing on any host withouttzdata-legacy, for exactly this reason.Test plan
cargo test --offline --lib— 806 passed, 0 failed. This branch is what makes the twoconfig::expiry_testscases pass; they fail on the base commit for want of the legacy timezone files.cargo check --offline --features python— clean.🤖 Generated with Claude Code