Closed Bug 1615060 Opened 5 years ago Closed 5 years ago

Add distribution search engine customisations to m-c

Categories

(Firefox :: Search, task, P3)

task
Points:
8

Tracking

()

RESOLVED FIXED
Firefox 77
Iteration:
77.2 - Apr 20 - May 3
Tracking Status
firefox77 --- fixed

People

(Reporter: daleharvey, Assigned: daleharvey)

References

Details

Attachments

(1 file)

We currently ship customised distributions of firefox with search engine overrides that are still using the openSearch.xml format

With modern config in place we can add these to m-c and have the configuration take into account the distribution

Priority: -- → P3

Converting to a meta, since I think this will be a collection of bugs to get this in place.

Keywords: meta
Summary: Add distribution search engine customisations to m-c → [meta] Add distribution search engine customisations to m-c
Depends on: 1616617
Assignee: nobody → dharvey
Iteration: --- → 76.1 - Mar 9 - Mar 22
Points: --- → 8
Blocks: 1619929
Keywords: meta
Summary: [meta] Add distribution search engine customisations to m-c → Add distribution search engine customisations to m-c
Blocks: 1203170
Iteration: 76.1 - Mar 9 - Mar 22 → 76.2 - Mar 23 - Apr 5

Hi Mathieu

So I have a patch ready for this @ https://phabricator.services.mozilla.com/D67231, however in the meantime the switch to remotesettings has landed which has brought up a workflow problem.

This patch is testing data that we are fetching via remoteSettings, that means if I land the patch now without the remoteSettings data updated then the tests will fail, we could update the remoteSettings data now, get that approved, wait for the dump to land on m-c (hope that the dump doesnt trigger any test failures), then land the code.

The dump is done twice a week which means a potential 4 day lead on patches, can that be done on request? I am also a little uncomfortable with the possibility that we have to upgrade the remoteSettings production data without having the full m-c test suite run against it, another issue I can think of its that it quite unweidly to develop new features if we need to develop them by editing data in remotesettings staging instead of editing a local json file.

I am assuming we arent the first to run into this and was wondering if you had any suggestions, off the top of my head having the "dump" be formatted and editable means we could edit the file, land the code + tests and as long as remotesettings doesnt update the "dump" until the changes have been pushed to production would solve 2 out of 3 of the problems, taking that idea further the "dump" could be the source of data, we approve and land on m-c and remoteSettings picks up those changes up, updates and pushes those changes to clients.

Mark has mentioned this may not work great with the remoteSettings approval workflow, being that we have an existing approval system in bugzilla I am not sure we need to prioritise using the remoteSettings one if there are reasons not to

Cheers

Flags: needinfo?(mathieu)

I'm not sure I fully understood the situation and your confusion, but I'll give a try at explaining what could be helpful here.

  • the dump in m-c can be updated manually in your commit with curl https://firefox.settings.services.mozilla.com/v1/buckets/main/collections/search-config/records > services/settings/dumps/main/search-config.json
  • it is really important that the dump comes from the server and not edited manually in m-c, because timestamps have to remain consistent between the local DBs and the remote server
  • the synchronization will overwrite the local records if the local timestamps are older. (Note that synchronization would be broken/no-op if the local data has newer timestamps than the server, since synchronization is only one way :))

If you're really stuck with a chicken and egg issue, where the new code needs the new data, but the new data would need the new code, then I can only recommend you to keep your code compatible with old data, or only make data changes that are retro compatible with old code (that is the case anyway isn't it).

Don't hesitate to schedule a quick chat if I misunderstood your question. My concentration is not at its best lately.

Flags: needinfo?(mathieu)

Thanks, I hadnt considered taking the dump directly off the server, I think that workflow could work well for us

One hopefully last issue, I have a bunch of changes I would like to test, they are all uploaded to staging, however there are already changes being tested on stage that may conflict and I dont want to have to be going through review every time I want to locally test a change. Is there any way to get a dump that I can load without actually commiting those changes?

Cheers

Flags: needinfo?(mathieu)
Iteration: 76.2 - Mar 23 - Apr 5 → 77.1 - Apr 6 - Apr 19

Thanks, I hadnt considered taking the dump directly off the server, I think that workflow could work well for us

Ok great!

[...] however there are already changes being tested on stage that may conflict and I dont want to have to be going through review every time I want to locally test a change. Is there any way to get a dump that I can load without actually commiting those changes?

I guess the most convenient workflow in that case is to stick your changes in the local DB manually. And if the amount of data to change is huge, in order to avoid authoring the data twice, you can use a JSON string in your local tests, and once ready publish it using the REST API.

For example (untested code, but you'll get the idea):

const data = ```[
  { "id": "record-1", ...},
  { "id": "record-2", ...},
]```; 

client = RemoteSettings("search-config")
await client.db.importBulk(data);

And then to publish them:

  • login in the Admin UI and copy the bearer token (button in the top bar)
  • and run this using kinto-http.js
const { KintoHttpClient } = ChromeUtils.import(
  "resource://services-common/kinto-http-client.js"
);

const data = [...];

const kinto = new KintoHttpClient("https://settings-writer.prod.mozaws.net/v1", {
  headers: {
    Authorization: bearerToken
  }
});

const result = await client.bucket("main-workspace").collection("search-config")
  .batch(batch => {
    for (const r of data) {
      batch.updateRecord(r);
    }
  });

Using the dev server or a local server is also an option...

Flags: needinfo?(mathieu)
Iteration: 77.1 - Apr 6 - Apr 19 → 77.2 - Apr 20 - May 3
Blocks: 1630980

Adrian

The patch for this bug contains some remoteSettings changes that I have pushed to production-preview.

I was wondering if you could do a smoketest of the configuration changes, not any of actual distribution changes, Mike Kaply is going to be checking those, but just to make sure that the configuration changes dont break anything normal. I think we should probably consider this being part of a standard process when pushing remoteSettings changes into production

I did a smoketest myself of these changes and found https://bugzilla.mozilla.org/show_bug.cgi?id=1631241, so in case you see that its not related to the configuration changes and is known, cheers

Flags: needinfo?(aflorinescu)

Sure Dale, thanks for flagging this. We're going to take a look.

As for
(In reply to Dale Harvey (:daleharvey) from comment #8)

I think we should probably consider this being part of a standard process when pushing remoteSettings changes into production

We already agreed with Mark(:standard8) to get a process in place for manually smoking prod-preview before hitting production, but that's a WIP and it's probably only going to be finished in time for Modernization hitting release - draft here - since it requires QA to have everything ready and a good handle of the modernization and its config (things we are still fumbling with).

So, meanwhile going by NI and bugzilla ticket would be good enough, I'd reckon.

Depends on: 1631740

Ok great thats good to hear, thanks, filed https://bugzilla.mozilla.org/show_bug.cgi?id=1631740

removing the NI, since comment 8 request got handled in bug 1631740.

Flags: needinfo?(aflorinescu)
Pushed by dharvey@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/67c929b22dc7 Add distribution engines to m-c r=Standard8,mkaply
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 77
Depends on: 1635168
Regressions: 1635168
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: