Closed
Bug 1000221
Opened 8 years ago
Closed 8 years ago
add support for "isOSUpdate" attribute
Categories
(Release Engineering Graveyard :: Applications: Balrog (backend), defect)
Release Engineering Graveyard
Applications: Balrog (backend)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: bhearsum, Assigned: bhearsum)
References
Details
Attachments
(4 files, 1 obsolete file)
4.12 KB,
patch
|
nthomas
:
review-
|
Details | Diff | Splinter Review |
1.88 KB,
patch
|
aki
:
review+
bhearsum
:
checked-in+
|
Details | Diff | Splinter Review |
3.73 KB,
patch
|
nthomas
:
review+
bhearsum
:
checked-in+
|
Details | Diff | Splinter Review |
7.41 KB,
patch
|
nthomas
:
review+
bhearsum
:
checked-in+
|
Details | Diff | Splinter Review |
b2g invented this attribute to distinguish FOTA from OTA updates. Sample XML: <updates> <update type="minor" appVersion="31.0a1" version="31.0a1" extensionVersion="31.0a1" buildID="20140422160202" licenseURL="http://www.mozilla.com/test/sample-eula.html" detailsURL="http://www.mozilla.com/test/sample-details.html" isOSUpdate="true"> <patch type="complete" URL="http://update.boot2gecko.org/hamachi/2.0.0/nightly/b2g_update_20140422160202.mar?build_id=20140422160202&version=31.0a1" hashFunction="SHA512" hashValue="ee608d3ad8693de788d3bb86f655e77444000c1654ef25b990a6217f29f8f1317c9942798c669972dcb45385b1d3a08fe1bcdbf6489d9cfad9b0d50fc1598e82" size="57652123"/> </update> </updates> This should be trivial to add on after bug 748698 is fixed, as it adds support for extended attributes in general, and this is purely additive (so we could add it to schema v2 AFAICT). If we need it before that bug is fixed, we'll have to hack it in somehow. I'd really, really prefer to wait though.
Assignee | ||
Comment 1•8 years ago
|
||
I have a WIP for the server and client side. Need to test it still: https://github.com/bhearsum/balrog/commit/59dee9c4634a0d552b1155b1066984c69beb0860 https://github.com/bhearsum/tools/commit/75cf33b853100da40e0499a9fba1248683ea6107 https://github.com/bhearsum/mozharness/commit/8e4f63c33a1f4a411cb3c8ade81ffcdecdd990dd
Assignee | ||
Comment 2•8 years ago
|
||
I wasn't sure which level of the blob to add this at. I mostly chose locale level because it made the submitter side of things easier. I remember you talking about maybe wanting to make all properties overridable at the platform/locale level, so I didn't spend too much time thinking about it... Anyways, I'm happy to move this to a different level if you'd prefer. It needs to be at least down as far as specific platforms to support hamachi and other devices in the same blob (and thus same rule) though.
Attachment #8431805 -
Flags: review?(nthomas)
Assignee | ||
Comment 3•8 years ago
|
||
Attachment #8431807 -
Flags: review?(aki)
Assignee | ||
Comment 4•8 years ago
|
||
Attachment #8431810 -
Flags: review?(nthomas)
Updated•8 years ago
|
Attachment #8431807 -
Flags: review?(aki) → review+
Assignee | ||
Updated•8 years ago
|
Attachment #8431807 -
Flags: checked-in+
Comment 5•8 years ago
|
||
Comment on attachment 8431810 [details] [diff] [review] isOSUpdate-tools.diff We'll add Release support if we ever need it ? Looks fine for the nightly case.
Attachment #8431810 -
Flags: review?(nthomas) → review+
Comment 6•8 years ago
|
||
Comment on attachment 8431810 [details] [diff] [review] isOSUpdate-tools.diff >diff --git a/lib/python/balrog/submitter/cli.py b/lib/python/balrog/submitter/cli.py >+ if isOSUpdate: >+ data['isOSUpdate'] = isOSUpdate > assert schemaVersion in (1, 2), 'Unhandled schema version %s' % schemaVersion > if schemaVersion == 1: > data['appv'] = appVersion > data['extv'] = extVersion > elif schemaVersion == 2: Actually, I'd suggest putting the new lines in the 'schemaVersion == 2' branch of the if block.
Comment 7•8 years ago
|
||
Comment on attachment 8431805 [details] [diff] [review] add isOSUpdate to schema v2 Code looks fine, but r- for missing tests. re the levels to support isOSUpdate - my attitude goes like this * for desktop releases, I'd like to use displayVersion etc at the top level to avoid needless duplication at the locale level * for nightlies, I accept we have to support locale level because of the staggered update of the latest blob Given we have multiple devices in B2G latest blobs, lets just go with locale level.
Attachment #8431805 -
Flags: review?(nthomas) → review-
Assignee | ||
Comment 8•8 years ago
|
||
(In reply to Nick Thomas [:nthomas] from comment #5) > Comment on attachment 8431810 [details] [diff] [review] > isOSUpdate-tools.diff > > We'll add Release support if we ever need it ? Looks fine for the nightly > case. Yeah. I don't see anything on the horizon that's going to require it, so I didn't bother. I could add it now if you'd like though. > >diff --git a/lib/python/balrog/submitter/cli.py b/lib/python/balrog/submitter/cli.py > >+ if isOSUpdate: > >+ data['isOSUpdate'] = isOSUpdate > > assert schemaVersion in (1, 2), 'Unhandled schema version %s' % schemaVersion > > if schemaVersion == 1: > > data['appv'] = appVersion > > data['extv'] = extVersion > > elif schemaVersion == 2: > > Actually, I'd suggest putting the new lines in the 'schemaVersion == 2' > branch of the if block. Good point. It would be an error to put this in the schema 1 blob. Perhaps that should be checked by balrog-submitter.py? (In reply to Nick Thomas [:nthomas] from comment #7) > Comment on attachment 8431805 [details] [diff] [review] > add isOSUpdate to schema v2 > > Code looks fine, but r- for missing tests. > > re the levels to support isOSUpdate - my attitude goes like this > * for desktop releases, I'd like to use displayVersion etc at the top level > to avoid needless duplication at the locale level > * for nightlies, I accept we have to support locale level because of the > staggered update of the latest blob > > Given we have multiple devices in B2G latest blobs, lets just go with locale > level. Sounds fine to me, thanks!
Assignee | ||
Comment 9•8 years ago
|
||
Attachment #8431810 -
Attachment is obsolete: true
Attachment #8433463 -
Flags: review?(nthomas)
Assignee | ||
Comment 10•8 years ago
|
||
Thanks for the reminder about the test.
Attachment #8433497 -
Flags: review?(nthomas)
Updated•8 years ago
|
Attachment #8433497 -
Flags: review?(nthomas) → review+
Updated•8 years ago
|
Attachment #8433463 -
Flags: review?(nthomas) → review+
Comment 11•8 years ago
|
||
Commit pushed to master at https://github.com/mozilla/balrog https://github.com/mozilla/balrog/commit/a0d686bd5bca667fb330592609ecf03d6803f5ff bug 1000221: add support for "isOSUpdate" attribute. r=nthomas
Assignee | ||
Updated•8 years ago
|
Attachment #8433497 -
Flags: checked-in+
Assignee | ||
Comment 12•8 years ago
|
||
Comment on attachment 8433463 [details] [diff] [review] with sanity check The backend patch landed in production today. I've landed this, so tomorrow blob's _should_ have all the necessary info to serve proper Hamachi updates.
Attachment #8433463 -
Flags: checked-in+
Assignee | ||
Comment 13•8 years ago
|
||
The latest submissions to Balrog look good: Hamachi is "isOSUpdate" set, and other devices do not. I'm looking for someone to help test an actual Hamachi device update today to verify. https://aus4.mozilla.org/update/3/B2G/32.0a1/20140515163731/hamachi/en-US/nightly/Boot2Gecko%202.0.0.0-prerelease/default/default/update.xml?force=1 http://update.boot2gecko.org/hamachi/2.0.0/nightly/update.xml ^ both are the same, except for the URL/version differences. Balrog uses newer version styles, and doesn't return the fake notes/license URLs.
Assignee | ||
Comment 14•8 years ago
|
||
QA also tested a real Hamachi build. They hit an unrelated issue, but the test otherwise worked. There was also a random person in #b2g that was able to update without any issues at all.
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Updated•2 years ago
|
Product: Release Engineering → Release Engineering Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•