Closed
Bug 1381617
Opened 7 years ago
Closed 7 years ago
Scalars.yaml expires: "Never" doesn't work but doesn't error
Categories
(Toolkit :: Telemetry, enhancement, P1)
Toolkit
Telemetry
Tracking
()
RESOLVED
FIXED
mozilla56
Tracking | Status | |
---|---|---|
firefox56 | --- | fixed |
People
(Reporter: benjamin, Assigned: Dexter)
Details
(Whiteboard: [measurement:client])
Attachments
(1 file, 2 obsolete files)
6.90 KB,
patch
|
Dexter
:
review+
|
Details | Diff | Splinter Review |
I was adding a new scalar and mis-capitalized the expiry in Scalars.yaml: plugin_instantiated: bug_numbers: - 1381591 description: >- The number of plugin instances that were created. expires: 'Never' # Jan-2021 but we don't have a version number for that kind: uint notification_emails: - bsmedberg@mozilla.com release_channel_collection: opt-out record_in_processes: - 'main' - 'content' This built correctly but didn't record anything. It would probably be good to add some stricter validation for this field so that the only things that are accepted are "never" lowercase, or a two-digit number.
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → alessio.placitelli
Points: --- → 1
Priority: -- → P1
Whiteboard: [measurement:client]
Assignee | ||
Comment 1•7 years ago
|
||
Chris, I found that the problem Benjamin mentioned also affects Histogram and events. I refactored the validation code in a shared function, which now consistently checks all the collection primitives.
Attachment #8887424 -
Flags: review?(chutten)
Comment 2•7 years ago
|
||
Comment on attachment 8887424 [details] [diff] [review] bug1381617.patch Review of attachment 8887424 [details] [diff] [review]: ----------------------------------------------------------------- One nit. ::: toolkit/components/telemetry/shared_telemetry_utils.py @@ +120,5 @@ > > +def validate_expiration_version(expiration): > + """ Makes sure the expiration version has the expected format. > + > + Allowed examples: "1.0", "20", "300.0a1", "60.0a1", "30.5a1", "never' mismatched quotes on "never'
Attachment #8887424 -
Flags: review?(chutten) → review+
Assignee | ||
Comment 3•7 years ago
|
||
Attachment #8887424 -
Attachment is obsolete: true
Attachment #8887527 -
Flags: review+
Assignee | ||
Comment 4•7 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=0b9d04bee7aa6ab9bd5e212ac8fb687a1c858bc6
Assignee | ||
Comment 5•7 years ago
|
||
Fix the linting problems.
Attachment #8887527 -
Attachment is obsolete: true
Attachment #8887547 -
Flags: review+
Assignee | ||
Comment 6•7 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/092390ed4c628d13875ddf3f9412e3de2b29a98c Bug 1381617 - Enable stricter scalar, histogram and events expiration validation. r=chutten
Pushed by alessio.placitelli@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/092390ed4c62 Enable stricter scalar, histogram and events expiration validation. r=chutten
Comment 8•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/092390ed4c62
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox56:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
You need to log in
before you can comment on or make changes to this bug.
Description
•