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

Status: RESOLVED FIXED

(AUS Graveyard :: General, defect)

Description

[Michael Morgan [:morgamic]]

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)

Comment 1

[Michael Morgan [:morgamic]]

Attachment: 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.

Comment 2

[Michael Morgan [:morgamic]]

This has been checked in -- working on staging it and testing for regressions.

Comment 3

[Michael Morgan [:morgamic]]

This has been pushed into production.

Comment 4

[Michael Morgan [:morgamic]]

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

Comment 5

[Michael Morgan [:morgamic]]

Some of us have recommended "-fallback-" as the identifier, since it's explicit.  Waiting for preed to say 'ok'.

Comment 6

[J. Paul Reed [:preed]]

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?

Comment 7

[Michael Morgan [:morgamic]]

(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

[J. Paul Reed [:preed]]

(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.

Comment 9

[Michael Morgan [:morgamic]]

Attachment: 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.

Comment 10

[Michael Morgan [:morgamic]]

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.

Comment 11

[Michael Morgan [:morgamic]]

Attachment: 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.

Comment 12

[J. Paul Reed [: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.

Comment 13

[Michael Morgan [:morgamic]]

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.

Comment 14

[Michael Morgan [:morgamic]]

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.

Comment 15

[Michael Morgan [:morgamic]]

Attachment: 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.

Comment 16

[J. Paul Reed [:preed]]

Comment on attachment 243524 [details] [diff] [review]
v3, with the ability to stop fallback

With the regex change discussed on IRC.

Comment 17

[Michael Morgan [:morgamic]]

Attachment: v4, for the record

This was the approved patch, w/ the adjusted regexp.

Comment 18

[Michael Morgan [:morgamic]]

Patch checked in and tagged for staging.