Say we have a partner build that has custom updates. This partner build is named "release-carebears18.104.22.168" and is on a channel of the same name. We need to update the AUS code so that we could have release-carebears22.214.171.124 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-carebears126.96.36.199 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)
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)
Attachment #229325 - Flags: first-review?(clouserw) → first-review+
This has been checked in -- working on staging it and testing for regressions.
This has been pushed into production.
Status: ASSIGNED → RESOLVED
Last Resolved: 13 years ago
Resolution: --- → FIXED
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 → ---
Some of us have recommended "-fallback-" as the identifier, since it's explicit. Waiting for preed to say 'ok'.
Status: REOPENED → ASSIGNED
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?
(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.
(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.
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 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.
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+
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 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-
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)
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+
This was the approved patch, w/ the adjusted regexp.
Patch checked in and tagged for staging.
Status: ASSIGNED → RESOLVED
Last Resolved: 13 years ago → 13 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.