Allow partner channels to fall-back to a more general channel

RESOLVED FIXED in 4.x (triaged)

Status

AUS Graveyard
General
--
major
RESOLVED FIXED
12 years ago
12 years ago

People

(Reporter: morgamic, Assigned: morgamic)

Tracking

4.x (triaged)

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 4 obsolete attachments)

(Assignee)

Description

12 years ago
Say we have a partner build that has custom updates.  This partner build is named "release-carebears1.5.0.5" and is on a channel of the same name.

We need to update the AUS code so that we could have release-carebears1.5.0.5 fall back to "release" (splitting on the -) in the event where we'd want to issue an update that was much broader than just the partner build's scope.

The logic will be adjusted so that findPatch() in the XML portion of AUS will do an extra check on the first part of the channel (.*)-.* -> $1 if the partner-specific update doesn't exist for the original channel.

In short:
a) first check release-carebears1.5.0.5
b) if nothing is there, check the release channel dir
c) if nothing is there, offer no updates (unless this is a nightly, then nightly logic will apply)
(Assignee)

Updated

12 years ago
Status: NEW → ASSIGNED
(Assignee)

Comment 1

12 years ago
Created attachment 229325 [details] [diff] [review]
Patch to add fall-back for partner channels.

Needs to be tested further -- but the idea is to fall back on a channel directory that is the base of the partner channel -- so foo-bar -> foo in the case where foo-bar doesn't exist in the build source directory.
Attachment #229325 - Flags: first-review?(clouserw)

Updated

12 years ago
Attachment #229325 - Flags: first-review?(clouserw) → first-review+
(Assignee)

Comment 2

12 years ago
This has been checked in -- working on staging it and testing for regressions.
(Assignee)

Comment 3

12 years ago
This has been pushed into production.
Status: ASSIGNED → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED
(Assignee)

Comment 4

12 years ago
This caused a regression in some other partner builds, so we are rolling this back to reevaluate our approach.  I believe a fix may be in:
a) finding another designation in the channel name besides - since "-" already exists for certain partner channels
b) making sure we copy or cc: build on all future AUS bugs
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 5

12 years ago
Some of us have recommended "-fallback-" as the identifier, since it's explicit.  Waiting for preed to say 'ok'.
Status: REOPENED → ASSIGNED

Comment 6

12 years ago
So, we talked about this more in IRC and I suggested:

/^(release|beta)(test)?\-cck\-\w+$/

morgamic's gonna make sure I got all the quoting/grouping right, but for now, we can go with that.

I might *also* suggest extra "whitelist" functionality, wherein we can explicitly list channels to not fallback, as part of this.

So, the logic would be something like:

1. Is this channel in the whitelist? If so, then normal behavior (i.e. if that exact channel name has no updates, offer no updates). If not, then...

2. Does this channel match the regex above? If not, normal behavior (see above); if so:

3. Does the channel have any updates? If so, offer them. If not, fall back (to... what... chop off everything after [and including] "-cck-")?

Does that sound doable/reasonable, morgamic?
(Assignee)

Comment 7

12 years ago
(In reply to co
> Does that sound doable/reasonable, morgamic?
Looks okay, except the whitelist check would come between step 2 and 3.  Shouldn't we assume that everything will take an update if the explicit build data for that update exists?

The fallback stuff only comes into play after the initial explicit update check fails.  Then we have two choices -- test for fallback and pursue that route (if the channel matches our regexp), then do the nightlies thing if the channel is a nightly channel.

Comment 8

12 years ago
(In reply to comment #7)
> (In reply to co
> > Does that sound doable/reasonable, morgamic?
> Looks okay, except the whitelist check would come between step 2 and 3. 
> Shouldn't we assume that everything will take an update if the explicit build
> data for that update exists?

Yah, that does make the logic much easier. :-)

What you said.
(Assignee)

Comment 9

12 years ago
Created attachment 232761 [details] [diff] [review]
Patch so we can push the -cck- fallback stuff in the short-term.

This should only key off -cck- so it should not affect existing channels with "-" in them.  Need to get this out so Basil and Juan can do their testing for their partner builds.  We can take a look at the whitelisting / blacklisting for fallback channels later.
Attachment #229325 - Attachment is obsolete: true
Attachment #232761 - Flags: first-review?(preed)
(Assignee)

Comment 10

12 years ago
Comment on attachment 232761 [details] [diff] [review]
Patch so we can push the -cck- fallback stuff in the short-term.

Gah -- you know what, it won't be much more effort to do what we talked about in the previous comment -- I think I will give that a shot.
Attachment #232761 - Attachment is obsolete: true
Attachment #232761 - Flags: first-review?(preed)
(Assignee)

Comment 11

12 years ago
Created attachment 232766 [details] [diff] [review]
Patch to include regexp, as discussed in preed's comment.

Okay, this should do the trick.  Also removed the split on -cck- for after the regexp check -- and moved returns outside of else statements.
Attachment #232766 - Flags: first-review?(preed)
Comment on attachment 232766 [details] [diff] [review]
Patch to include regexp, as discussed in preed's comment.

This looks sane; one nit I'd like to see fixed pre-checkin: make the fallback regex a constant up above somewhere, so it's not buried in the code.
Attachment #232766 - Flags: first-review?(preed) → first-review+
(Assignee)

Comment 13

12 years ago
After more discussion, the fallback mechanism needs a 'stopper' to prevent fallbacks for given fallback channels.  Problems could arise where we have a release update, but the partner update is not ready yet, and we have to push the release update -- but we don't want to update a partner to the release.

In short, we need a way to stop the default behavior from happening.  Ways discussed:
* Adding zero-byte znippet files in the parter directories to signify "don't update"
* Adding config options for "channels allowed to fallback" forcing us to make a manual, deliberate choice to fallback

A lot of these relationships would be better defined in an RDBMS... something to keep in mind.
(Assignee)

Comment 14

12 years ago
Comment on attachment 232766 [details] [diff] [review]
Patch to include regexp, as discussed in preed's comment.

This lacks a way to stop a fallback update.
Attachment #232766 - Flags: first-review+ → first-review-
(Assignee)

Comment 15

12 years ago
Created attachment 243524 [details] [diff] [review]
v3, with the ability to stop fallback

This patch is against the production tag, and check to make sure the explicit update doesn't have zero-byte files.  If it does, fallback would be stopped.  This is currently independent of the build comparison bug.
Attachment #243524 - Flags: first-review?(preed)
(Assignee)

Updated

12 years ago
Attachment #232766 - Attachment is obsolete: true
Comment on attachment 243524 [details] [diff] [review]
v3, with the ability to stop fallback

With the regex change discussed on IRC.
Attachment #243524 - Flags: first-review?(preed) → first-review+
(Assignee)

Comment 17

12 years ago
Created attachment 243548 [details] [diff] [review]
v4, for the record

This was the approved patch, w/ the adjusted regexp.
Attachment #243524 - Attachment is obsolete: true
Attachment #243548 - Flags: first-review+
(Assignee)

Comment 18

12 years ago
Patch checked in and tagged for staging.
Status: ASSIGNED → RESOLVED
Last Resolved: 12 years ago12 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.