Repository navigation
Observe the audience collection, not just one audience - #91
Conversation
observeList() fired only on lists/{id} and kept the first audience the
process saw. An install with four store views therefore described one of
its four, and nothing said how many existed -- so coverage was a
tautology, audiences we hold over audiences we hold, and the question
that matters, whether an audience we never saw has changed, could not be
asked at all.
The collection answers both halves in one response: total_items is the
count, and each entry carries the same stats a single-audience read does.
Nothing new is requested -- the admin fetches the collection when it
offers the audience dropdown.
So this adds observeListCollection() for `lists`, keeps observeList() for
lists/{id}, and records both into a map keyed by audience id. A later
reading of the same audience completes an earlier one, which is what lets
a single-audience response carrying total_contacts fill an entry the
collection left without it.
The flat list_* fields are kept beside the map rather than replaced by
it: a consumer reading them predates the map and must not lose its value
to this change.
The count is never capped; only the array is, at MAX_AUDIENCES. An
account with hundreds would otherwise push the report past MAX_BYTES and
have it dropped whole -- losing the denominator, which is the one figure
nothing else can supply. audiences_reported counts every audience
offered rather than every one carried, so truncation is visible by
comparing it with the array's length and there is no flag to forget.
Measured: 24 real audiences produce a 2,760-byte report; 140 synthetic
ones cap at 50 and produce 5,303 against a MAX_BYTES of 8,192.
|
The design is right — the collection as the source, the map keyed by audience id so a later reading completes an earlier one, keeping the flat fields beside it rather than replacing them, and refusing to cap the count. All four are the calls I would have argued for. But the key names are wrong, and that is on me. The table you worked from listed storage names, not the names the wire expects. They are not the same and I should have said which I was giving you. Here is the actual contract. The wire contractTwo keys, both short:
Inner keys are abbreviated too — "la": { "3fd8ea8578": {"mem":1120,"uns":1,"cln":631,"tot":1767}, "c75ded2746": {"mem":0} }
"lac": 24The array form is equally valid and reads the id off the entry: "la": [ {"id":"3fd8ea8578","mem":1120,"uns":1,"cln":631,"tot":1767} ]Prefer the map. JSON object keys are unique, so a duplicate id is impossible by construction rather than resolved by a rule — in the array form a repeated id is dropped, first-wins. Any of the four counts may be absent: a missing one is simply unknown for that audience and does not cost you the entry. An entry without a usable id is dropped, so the id is the only mandatory part. Ids are bounded at 32 characters, matching Two bounds to changeCap the roster at 12, not 50. Also keep the serialised roster under 4,096 bytes. Over that it is dropped whole rather than stored oversized, so the cap is what protects you from the all-or-nothing case you were already designing against with Drop You are right about
|
Raised in review: the names I worked from were storage names rather than the ones the wire expects. The roster goes as `la`, keyed by audience id, with `mem`, `uns`, `cln` and `tot` inside. The account-wide count goes as `lac`. A map rather than a list because object keys are unique, so a repeated id is impossible by construction instead of resolved by a rule nobody reads. audiences_reported is dropped. The distinction it drew -- offered against carried -- already exists: `lac` is the offered count and the roster's size is the carried one, so a third key adds nothing and gives something else to forget to set. The cap falls from fifty to twelve. An unpaged GET /lists answers ten, so twelve carries a default observation whole and leaves headroom, and the receiver refuses an oversized roster rather than truncating it -- which would cost the count as well, and that is the figure nothing else can supply. A byte bound backs the cap up for ids longer than any seen, and trims one entry at a time, because carrying fewer audiences is strictly better than carrying none. Measured: ten audiences serialise to 472 bytes and twelve to 570, against a bound of 4,096.
dipola
left a comment
dipola
left a comment
There was a problem hiding this comment.
Approved. Checked against the contract rather than the description: la and lac on the envelope, mem / uns / cln / tot inside, mapped from Mailchimp's own longer names at the point the response is read — which is the right place for the translation to live. Cap down to twelve, audiences_reported gone.
The 4,096-byte guard was not something I asked for and it is the best part of the change. An oversized roster is refused whole at the far end, so bounding it here is what keeps the count — the figure nothing else can supply — from going down with it.
|
Both merged — Holding both tags until you answer one question, because your line on the collection PR has a The order is not symmetricThe bad state you described — the collection observed, answering without contact totals — is The installs that matter here are the ones running old extensions, and they pick up library releases So tagging the library first opens that window over exactly the population it hurts most. Tagging the The questionDoes your console distinguish an entry with no You wrote that any of the four counts may be absent and that a missing one is simply unknown for that If absent collapses into zero anywhere on the read path, the order does matter, and I would tag Nothing is blocked on my side: both releases are prepared and I will cut them as soon as you answer. |
Part 2 of the library-side telemetry ask — the denominator, the per-audience map, and the cap.
What was missing
observeList()fired only onlists/{id}and kept the first audience the process saw. A four-store-view install described one of its four, and nothing said how many existed. Coverage was a tautology — audiences we hold over audiences we hold — and the question that actually matters, has an audience we never saw changed, could not be asked.The collection answers both halves at once
Checked against a live account before designing on it:
So one response carries the count and every audience's stats. Nothing new is requested — the admin fetches this collection when it offers the audience dropdown.
Shape
observeListCollection()forlists,observeList()forlists/{id}— the guard added last week stays, and the collection gets its own entry point rather than loosening it.total_contactsfill an entry the collection left without it.list_*fields are kept beside the map, not replaced by it. A consumer reading them predates this change and should not lose its value to it.The cap, and what is deliberately not capped
MAX_AUDIENCES = 50bounds the array. The count is never capped — an account with hundreds would otherwise push the report pastMAX_BYTESand have it dropped whole, losing the denominator, which is the one figure nothing else can supply. A truncated array beside an intact count still answers the coverage question.audiences_reportedis the number offered, not the number carried, so truncation shows asaudiences_reported > count(audiences_json)and there is no flag to forget to set.Verified
audience_count=24,reported=24, array 24, envelope 2,760 bytes{"id":"…","member_count":2,"unsubscribe_count":0,"cleaned_count":0,"total_contacts":6}audience_count=140,reported=140, envelope 5,303 againstMAX_BYTES8,192tools/no-deprecated-calls.php: 64 files, 0 findings.Two things still open
The key names, which are yours to set — I will match whatever the contract says.
It changes a decision on mc-magento2 #2366. I left
Controller/Adminhtml/Lists/Get.phpalone there,arguing the beacon ignores collection responses so asking would add payload nobody reads. That argument
dies with this PR: the collection becomes the richest source, and without the parameter its entries
arrive without the contact total.