Closed Bug 507408 Opened 11 years ago Closed 10 years ago

AUS should pass the ?force=1 flag through to bouncer

Categories

(AUS Graveyard :: Systems, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: dre, Assigned: morgamic)

References

Details

Attachments

(3 files)

When a user manually checks for updates by clicking "Help | Check for updates...", the request to AUS has a flag in it, "force=1" to indicate it was a manual check.  Metrics needs this flag to also be included in the request to bouncer for the download so that we can track the difference between automatic update downloads and manually initiated downloads.
I mentioned to Morgamic that I would really like to get this patch into AUS before an automatic notification of Major Update for 3.5.2 is released.  Otherwise, we'll have no ability to properly categories those upgrades.
Hardware: x86 → All
How do you want the bouncer url to work?  Just add &force=1?
Yep, that will be perfect.
Ok, will get to it early next week.
Status: NEW → ASSIGNED
Comment on attachment 393248 [details] [diff] [review]
v1, passthru for force=1 and updated acceptance tests

This looks fine...but this is the first time I've ever seen this code, so take that FWIW!
Attachment #393248 - Flags: review?(bhearsum) → review+
Need to add logic to support both ?force=1 and &force=1 cases.
Attachment #439368 - Flags: review?(bhearsum)
This was checked in on trunk.  Tests pass and URLs are valid.
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
(In reply to comment #8)
> Created an attachment (id=439368) [details]
> follow-up patch, adds check for ? to make sure we don't build invalid URLs

There was an additional change to Verify.txt that landed with this:
http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&subdir=mozilla/webtools/aus/tests&command=DIFF_FRAMESET&file=Verify.txt&rev1=1.13&rev2=1.14&root=/cvsroot

And it fails for me, presumably because 
 tests/data/3/Synthetic/1.0/platform/1000000002/locale/bouncerchannel
only has a CVS dir. Perhaps a forgotten 'cvs add' ?
txt files were missing -- added those to trunk.
A bouncer url is always going to have a ?product=foo on it so I'm wondering what lead to comment #7 and 
 http://mxr.mozilla.org/mozilla/source/webtools/aus/xml/inc/xml.class.php#114

I ask because on the beta channel we don't use bouncer and point directly at ftp.m.o. Eg: https://aus2.mozilla.org/update/3/Firefox/3.6.3/20100401064631/Darwin_Universal-gcc3/en-US/beta/Darwin%2010.3.0/default/default/update.xml?force=1
points to http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/3.6.4-candidates/build4/update/mac/en-US/firefox-3.6.4.complete.mar?force=1
That still works if I use curl, but I'd like to confirm that Apache will reliably drop the option without us needing a special config there.
We could alternatively drop it for anything that isn't a bouncer URL.
Like this perhaps. Passes the tests.
Attachment #447953 - Flags: review?(morgamic)
Regarding comment #12,  If we don't use bouncer for beta downloads, then that means we can't see / don't track downloads for beta channel at all.  That surprises me.  Has that always been the case?
Comment on attachment 447953 [details] [diff] [review]
Only append '&force=1' to bouncer urls

This works.  Thanks Nick.
Attachment #447953 - Flags: review?(morgamic) → review+
Daniel - as far as I know yes.  RC's I think get distributed over bouncer, though?
Comment on attachment 447953 [details] [diff] [review]
Only append '&force=1' to bouncer urls

Landed with a slight tweak to the description of the bouncerchannel test:

Checking in tests/Verify.txt;
/cvsroot/mozilla/webtools/aus/tests/Verify.txt,v  <--  Verify.txt
new revision: 1.16; previous revision: 1.15
done
Checking in xml/inc/xml.class.php;
/cvsroot/mozilla/webtools/aus/xml/inc/xml.class.php,v  <--  xml.class.php
new revision: 1.6; previous revision: 1.5
done
'beta' is so overloaded, 'scuse the verbosity ...

(In reply to comment #15)
> Regarding comment #12,  If we don't use bouncer for beta downloads, then that
> means we can't see / don't track downloads for beta channel at all.  That
> surprises me.  Has that always been the case?

In the period before a new release we've pointed people on the beta update channel to ftp.m.o because we
 * don't want to have to wait for the mirrors to sync
 * don't need the mirror capacity anyway
 * felt uncomfortable about distributing pre-release builds more widely

As the count of users on beta is now around 7 figures (across all versions) we're starting to think about how to we can use the mirrors while still remaining nimble. And the network woes of the last week meant updates to 3.6.4 build6 were served via bouncer so we didn't stress the San Jose datacenter further. This is still ad-hoc, but may get adopted into the release process.

(In reply to comment #17)
> Daniel - as far as I know yes.  RC's I think get distributed over bouncer,
> though?

In general, any updates or installers that are going to cause significant traffic will be set up in bouncer. Things like 3.6RC1 definitely fall into that category. In practice that means just about everything except these beta channel updates prior to general release.

3.6.4 has been unusual, as it's the first time we've backported a major new feature to a stable branch, and the installers have been offered as '3.6.4 betas' on mozilla.com at
  http://www.mozilla.com/en-US/firefox/all-beta.html
That's via bouncer so we are counting them, the current product is Firefox-3.6.4build6.
Daniel, do you need to make any changes to the metrics infra before we deploy this change ?
Yes.  We should file a Metrics bug to add an attribute to the downloads data mart to track which requests have this parameter set.
That does not need to be done before this change is deployed though.
Depends on: 569160
Blocks: 569160
No longer depends on: 569160
Tagged and deployed in bug 569160.
You need to log in before you can comment on or make changes to this bug.