Closed
Bug 1266235
Opened 8 years ago
Closed 8 years ago
Rename Kinto blocklist clients internal preferences
Categories
(Cloud Services :: Firefox: Common, defect)
Cloud Services
Firefox: Common
Tracking
(firefox49 fixed)
RESOLVED
FIXED
Tracking | Status | |
---|---|---|
firefox49 | --- | fixed |
People
(Reporter: leplatrem, Assigned: leplatrem)
References
Details
Attachments
(4 files)
From Bug 1264914 https://bugzilla.mozilla.org/show_bug.cgi?id=1264914#c5 The preferences names used by the Kinto blocklist client can be improved: * Add a `blocklist` prefix since we may have several places in firefox relying on kinto (eg. url, bucket and collection names) * Replace the word `kinto` to something generic like `storage`, in case the product gets rebranded or replaced
Assignee | ||
Updated•8 years ago
|
Comment 1•8 years ago
|
||
Do we think that we will only use Kinto for blocklist features? The use case I had in mind was password manager recipes which isn't really a blocklist.
Assignee | ||
Comment 2•8 years ago
|
||
> Do we think that we will only use Kinto for blocklist features?
No, quite the opposite!
I must have formulated it badly: we shall add a prefix to the current preferences since they apply only for the blocklist.
For example, in `services/common/KintoBlocklist.js`, we currently have:
"services.kinto.base"
"services.kinto.bucket"
They should probably become something like:
"services.storage.blocklist.base"
"services.storage.blocklist.bucket"
So that we can have another dedicated pref for the base url and bucket used for other use-cases like the password manager recipes (cool by the way! I'm looking forward to knowing more some day :))
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → mathieu
Assignee | ||
Comment 3•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/48455/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/48455/
Attachment #8744319 -
Flags: review?(mgoodwin)
Attachment #8744320 -
Flags: review?(mgoodwin)
Assignee | ||
Comment 4•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/48457/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/48457/
Assignee | ||
Comment 5•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/48459/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/48459/
Assignee | ||
Comment 6•8 years ago
|
||
https://reviewboard.mozilla.org/r/48459/#review45165 ::: addon-sdk/source/test/preferences/no-connections.json:31 (Diff revision 1) > "extensions.update.url": "http://localhost/extensions-dummy/updateURL", > "extensions.update.background.url": "http://localhost/extensions-dummy/updateBackgroundURL", > "extensions.blocklist.url": "http://localhost/extensions-dummy/blocklistURL", > "extensions.webservice.discoverURL": "http://localhost/extensions-dummy/discoveryURL", > "extensions.getAddons.maxResults": 0, > - "services.kinto.base": "http://localhost/dummy-kinto/v1", > + "services.blocklist.base": "http://localhost/dummy-kinto/v1", This could be `services.settings.server` IMO
Assignee | ||
Comment 7•8 years ago
|
||
Related: * https://bugzilla.mozilla.org/show_bug.cgi?id=1248557 * https://bugzilla.mozilla.org/show_bug.cgi?id=1259947 Review commit: https://reviewboard.mozilla.org/r/49705/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/49705/
Assignee | ||
Comment 8•8 years ago
|
||
Comment on attachment 8744319 [details] MozReview Request: Bug 1266235 - Rename kinto-updater to blocklist-updater,r=mgoodwin Review request updated; see interdiff: https://reviewboard.mozilla.org/r/48455/diff/1-2/
Assignee | ||
Comment 9•8 years ago
|
||
Comment on attachment 8744320 [details] MozReview Request: Bug 1266235 - Rename KintoBlocklist to blocklist-clients,r=mgoodwin Review request updated; see interdiff: https://reviewboard.mozilla.org/r/48457/diff/1-2/
Assignee | ||
Comment 10•8 years ago
|
||
Comment on attachment 8744321 [details] MozReview Request: Bug 1266235 - Use blocklist prefix in preference names Review request updated; see interdiff: https://reviewboard.mozilla.org/r/48459/diff/1-2/
Comment 11•8 years ago
|
||
(In reply to Mathieu Leplatre (:leplatrem) from comment #7) > Created attachment 8747050 [details] > MozReview Request: Bug 1266235 - Move blocklist preferences to all.js Shouldn't security.onecrl.maximum_staleness_in_seconds be moved to all.js as well? I can't find it in your patch, only its reference for extensions.blocklist.interval.
Assignee | ||
Updated•8 years ago
|
Attachment #8744319 -
Flags: review?(MattN+bmo)
Attachment #8744320 -
Flags: review?(MattN+bmo)
Attachment #8744321 -
Flags: review?(MattN+bmo)
Attachment #8747050 -
Flags: review?(MattN+bmo)
Comment 12•8 years ago
|
||
Comment on attachment 8744319 [details] MozReview Request: Bug 1266235 - Rename kinto-updater to blocklist-updater,r=mgoodwin https://reviewboard.mozilla.org/r/48455/#review47019
Attachment #8744319 -
Flags: review?(mgoodwin) → review+
Comment 13•8 years ago
|
||
Comment on attachment 8744320 [details] MozReview Request: Bug 1266235 - Rename KintoBlocklist to blocklist-clients,r=mgoodwin https://reviewboard.mozilla.org/r/48457/#review47021
Attachment #8744320 -
Flags: review?(mgoodwin) → review+
Comment 14•8 years ago
|
||
Comment on attachment 8744319 [details] MozReview Request: Bug 1266235 - Rename kinto-updater to blocklist-updater,r=mgoodwin https://reviewboard.mozilla.org/r/48455/#review47619
Attachment #8744319 -
Flags: review?(MattN+bmo) → review+
Comment 15•8 years ago
|
||
Comment on attachment 8744320 [details] MozReview Request: Bug 1266235 - Rename KintoBlocklist to blocklist-clients,r=mgoodwin https://reviewboard.mozilla.org/r/48457/#review47621
Attachment #8744320 -
Flags: review?(MattN+bmo) → review+
Comment 16•8 years ago
|
||
Comment on attachment 8744321 [details] MozReview Request: Bug 1266235 - Use blocklist prefix in preference names https://reviewboard.mozilla.org/r/48459/#review47623
Attachment #8744321 -
Flags: review?(MattN+bmo) → review+
Comment 17•8 years ago
|
||
Comment on attachment 8747050 [details] MozReview Request: Bug 1266235 - Move blocklist preferences to all.js https://reviewboard.mozilla.org/r/49705/#review47627 ::: modules/libpref/init/all.js:2089 (Diff revision 1) > + > +// For now, let's keep settings server update out of the release channel > #ifdef RELEASE_BUILD > +pref("services.blocklist.update_enabled", false); "out of release builds" since release builds include more than the "release channel" e.g. betas
Attachment #8747050 -
Flags: review?(MattN+bmo) → review+
Assignee | ||
Comment 18•8 years ago
|
||
Comment on attachment 8747050 [details] MozReview Request: Bug 1266235 - Move blocklist preferences to all.js Review request updated; see interdiff: https://reviewboard.mozilla.org/r/49705/diff/1-2/
Assignee | ||
Comment 19•8 years ago
|
||
(In reply to rsx11m from comment #11) > (In reply to Mathieu Leplatre (:leplatrem) from comment #7) > > Created attachment 8747050 [details] > > MozReview Request: Bug 1266235 - Move blocklist preferences to all.js > > Shouldn't security.onecrl.maximum_staleness_in_seconds be moved to all.js as > well? > I can't find it in your patch, only its reference for > extensions.blocklist.interval. Yes, I think so. Let's move it too.
Comment 20•8 years ago
|
||
Ok, I'm holding back on bug 1269773 then until this checks in. (In reply to Mathieu Leplatre (:leplatrem) from comment #18) > https://reviewboard.mozilla.org/r/49705/diff/1-2/ This reads now "out of the release build" which should grammatically either be "out of [the] release builds" (plural, where I'm not sure if a "the" is needed here) or "out of a release build" (singular).
Assignee | ||
Comment 21•8 years ago
|
||
Comment on attachment 8747050 [details] MozReview Request: Bug 1266235 - Move blocklist preferences to all.js Review request updated; see interdiff: https://reviewboard.mozilla.org/r/49705/diff/2-3/
Assignee | ||
Updated•8 years ago
|
Keywords: checkin-needed
Comment 22•8 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/93deaa5641bc https://hg.mozilla.org/integration/fx-team/rev/5f321f10da1e https://hg.mozilla.org/integration/fx-team/rev/f560cba61749 https://hg.mozilla.org/integration/fx-team/rev/f684fac95bd9
Keywords: checkin-needed
I had to back this out for xpcshell failures like http://archive.mozilla.org/pub/firefox/tinderbox-builds/fx-team-win32-debug/1462880591/fx-team_xp_ix-debug_test-xpcshell-bm119-tests1-windows-build25.txt.gz (Search the page for `<<<<` and scroll up a bit to find the actual failure, `>>>>` and scroll up a bit to see what Treeherder says the failure is.) https://hg.mozilla.org/integration/fx-team/rev/ee562525573f
Flags: needinfo?(mathieu)
Assignee | ||
Comment 24•8 years ago
|
||
I am truely sorry for this. I've got a patch[0], what is the process now to submit it? Should backout your backout commit? Thanks for your patience :/ [0] https://pastebin.mozilla.org/8870913
Flags: needinfo?(wkocher)
Flags: needinfo?(mathieu)
Flags: needinfo?(MattN+bmo)
If you get the new patch reviewed, set another needinfo for me, I can reland the original ones and then land your new one on top of it.
Flags: needinfo?(wkocher)
Comment 26•8 years ago
|
||
Yeah, just push to mozreview again with the fixes in the appropriate commits (part 3 I think) then add checkin-needed as I'm fine with the changes.
Flags: needinfo?(MattN+bmo)
Assignee | ||
Comment 27•8 years ago
|
||
Comment on attachment 8744319 [details] MozReview Request: Bug 1266235 - Rename kinto-updater to blocklist-updater,r=mgoodwin Review request updated; see interdiff: https://reviewboard.mozilla.org/r/48455/diff/2-3/
Assignee | ||
Comment 28•8 years ago
|
||
Comment on attachment 8744320 [details] MozReview Request: Bug 1266235 - Rename KintoBlocklist to blocklist-clients,r=mgoodwin Review request updated; see interdiff: https://reviewboard.mozilla.org/r/48457/diff/2-3/
Assignee | ||
Comment 29•8 years ago
|
||
Comment on attachment 8744321 [details] MozReview Request: Bug 1266235 - Use blocklist prefix in preference names Review request updated; see interdiff: https://reviewboard.mozilla.org/r/48459/diff/2-3/
Assignee | ||
Comment 30•8 years ago
|
||
Comment on attachment 8747050 [details] MozReview Request: Bug 1266235 - Move blocklist preferences to all.js Review request updated; see interdiff: https://reviewboard.mozilla.org/r/49705/diff/3-4/
Assignee | ||
Comment 31•8 years ago
|
||
I rebased and submitted a Try build, and test are now ok. https://treeherder.mozilla.org/#/jobs?repo=try&revision=63aa3b6b8590
Assignee | ||
Updated•8 years ago
|
Keywords: checkin-needed
Comment 32•8 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/c25b2f3a7ce9 https://hg.mozilla.org/integration/fx-team/rev/04cd17b0025c https://hg.mozilla.org/integration/fx-team/rev/90ef5dbbb7c4 https://hg.mozilla.org/integration/fx-team/rev/085a53f65ff3
Keywords: checkin-needed
Comment 33•8 years ago
|
||
Backed out for browser_aboutCertError.js failures. https://hg.mozilla.org/integration/fx-team/rev/aa14a4a7c5df https://treeherder.mozilla.org/logviewer.html#?job_id=9331350&repo=fx-team#L1669
Assignee | ||
Comment 34•8 years ago
|
||
Comment on attachment 8744319 [details] MozReview Request: Bug 1266235 - Rename kinto-updater to blocklist-updater,r=mgoodwin Review request updated; see interdiff: https://reviewboard.mozilla.org/r/48455/diff/3-4/
Assignee | ||
Comment 35•8 years ago
|
||
Comment on attachment 8744320 [details] MozReview Request: Bug 1266235 - Rename KintoBlocklist to blocklist-clients,r=mgoodwin Review request updated; see interdiff: https://reviewboard.mozilla.org/r/48457/diff/3-4/
Assignee | ||
Comment 36•8 years ago
|
||
Comment on attachment 8744321 [details] MozReview Request: Bug 1266235 - Use blocklist prefix in preference names Review request updated; see interdiff: https://reviewboard.mozilla.org/r/48459/diff/3-4/
Assignee | ||
Comment 37•8 years ago
|
||
Comment on attachment 8747050 [details] MozReview Request: Bug 1266235 - Move blocklist preferences to all.js Review request updated; see interdiff: https://reviewboard.mozilla.org/r/49705/diff/4-5/
Assignee | ||
Comment 38•8 years ago
|
||
I rebased again and submitted a Try build. A very few tests are failing for weird reasons. But :arai said they are known to be intermittent. https://treeherder.mozilla.org/#/jobs?repo=try&revision=a2b68d58de1e (I could not retrigger the failing tests, I was prompted for credentials and mine are rejected) I'd take the risk and ask for check-in again...
Assignee | ||
Updated•8 years ago
|
Keywords: checkin-needed
Comment 39•8 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/3ffcd4e5783a https://hg.mozilla.org/integration/fx-team/rev/dc96a1440e3c https://hg.mozilla.org/integration/fx-team/rev/c6c57d394549 https://hg.mozilla.org/integration/fx-team/rev/a5aad78f70ea
Keywords: checkin-needed
Comment 40•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/3ffcd4e5783a https://hg.mozilla.org/mozilla-central/rev/dc96a1440e3c https://hg.mozilla.org/mozilla-central/rev/c6c57d394549 https://hg.mozilla.org/mozilla-central/rev/a5aad78f70ea
You need to log in
before you can comment on or make changes to this bug.
Description
•