WIP: Bug 1386307 - Expose Nightly updates status to public website #601
Conversation
There was a problem hiding this comment.
That's a wonderful start, @allan-silva !
I don't have a lot of feedback on the Nix part. I'm not familiar with it. Nonetheless, I managed to run you PR without any issue. Huzzah Nix, the please command and your config 😃 . @garbas may have a better feedback once he's back from PTO (in October).
To me, it looks good overall. The first API calls are correct. I'm looking forward to seeing the next item on the TODO list.
Thank you very much for this hard work, @allan-silva 😃
| defaults={'rule_alias': app.config.get('DEFAULT_ALIAS'), 'product': None, 'channel': None}, | ||
| view_func=ChannelStatusView.as_view('channel_status_default')) | ||
|
|
||
| app.add_url_rule('/<rule_alias>/status', |
There was a problem hiding this comment.
Nit: As the app is called "channel status", I think /status is redundant. /<rule_alias> or /<product>/<channel> sounds explanatory enough, to me
| import backend_common | ||
|
|
||
|
|
||
| # @pytest.fixture(scope='session') |
There was a problem hiding this comment.
@garbas may have more input about how to not use a DB, for this project
There was a problem hiding this comment.
Ah, I have removed database from settings.py and from app extensions. The code was commented because I did not start the tests yet.
| rule = self._get_rule(rule_alias, product, channel) | ||
| if not rule: | ||
| abort(404) | ||
| release = self._get_release(rule['mapping']) |
There was a problem hiding this comment.
Good idea on fetching the release blob 👍
It's actually needed if you want to know what's the build ID of "Firefox-mozilla-central-nightly-latest". I hadn't thought of that in bug 1386307.
|
My last commit basically puts information in a page, I'll need orientation to create UI in the right way. The default route will fetch nightly update by alias There is a route that supports product/channel query, using the following test route will fetch an example of froozen update: The page will warn when the update is throttled and my intention is show the both updates (mapping and fallbackMapping). Waiting for review from @JohanLorenzo or @garbas and other interested people. Update:
|
garbas
changed the title
There was a problem hiding this comment.
Thank you so much for the effort you put in this PR, Allan! Sorry for the delay, by the way :/
The technical basis is great! You wrote modular code that is easy to read and grasp. I really like it!
Overall, I think we should focus on Nightly. I see this page as a goto for nightly users when they suspect their nightly is too old. @pascalchevrel can testify that users are asking questions after a day or two, without updates.
I don't think we should overwhelm them with too much information. Just the current BuildId and a bug number should be enough. In my opinion, that's what makes a difference between this PR and https://mozilla.github.io/delivery-dashboard/#pollbot/firefox/58.0a1. The latter is for the Firefox Release Management team who wants to track every moving part of every channel.
Moreover, as far I know, beta or release users don't care as much about updates. In other words, I don't think it's worth to put too much effort on other channels than Nightly, for now.
What do you think @allan-silva ?
|
|
||
| {% if channel_status.is_latest_build_update %} | ||
| <div class="alert alert-success" role="alert"> | ||
| <span class="text-capitalize"> {{ channel_status.channel }}</span> updates are currently. |
There was a problem hiding this comment.
Nit 1: I realize a better sentence can be {{ channel_status.channel }} channel is currently. Sorry for bad wording of mine in bug 1386307
Nit 2: I think {{ channel_status.channel }} channel is currently should be displayed outside of the alert div. The important part of the sentence is serving the latest nightly ([Build ID]). So in my opinion, that's what should be in the alert div.
Nit 3: My apologies, I left a confusion about what's in the if and in the else. I believe the logic should be:
<span class="text-capitalize"> {{ channel_status.channel }}</span> updates are currently
{% if channel_status.is_latest_build_update %}
<div class="alert alert-success" role="alert">
<h2 class="alert-heading">serving the latest {{ channel_status.channel }} {{channel_status.rule['buildID']}}</h2>
</div>
{% else %}
<div class="alert alert-warning" role="alert">
<h2 class="alert-heading">frozen to {{channel_status.rule['buildID']}}</h2>
<p>Reason: {{ channel_status.rule['comment'] }}</p>
</div>
{% endif %}There was a problem hiding this comment.
I'm not sure if the buildID of the rule is the right place to get the latest update buildID, the current nightly has no buildID:
{ alias: "firefox-nightly", backgroundRate: 100, buildID: null, buildTarget: null, channel: "nightly", comment: null, data_version: 74, distVersion: null, distribution: null, fallbackMapping: null, headerArchitecture: null, instructionSet: null, locale: null, mapping: "Firefox-mozilla-central-nightly-latest", memory: null, mig64: null, osVersion: null, priority: 90, product: "Firefox", rule_id: 3, update_type: "minor", version: null }
There was a problem hiding this comment.
@JohanLorenzo
Should we consider the throttle in this page, or just look to mapping and not to fallbackMapping?
There was a problem hiding this comment.
Should we consider the throttle in this page, or just look to mapping and not to fallbackMapping?
Yes, the throttle is important. As you may know, a rule with:
backgroundRate: 0,
mapping: "Firefox-mozilla-central-nightly-latest",
fallbackMapping: "Firefox-mozilla-central-nightly-20171015220106",won't automatically serve the latest nightly. Then, we can't only rely on the mapping.
I'm not sure if the buildID of the rule is the right place to get the latest update buildID,
You're right, getting the buildID of "Firefox-mozilla-central-nightly-latest" is tricky, actually. I initially thought of fetching https://aus-api.mozilla.org/api/v1/releases/Firefox-mozilla-central-nightly-latest and looking into all locales in all platforms to see if one buildId stood out. But that's not a good idea, after all. This blob keeps changing during the day, depending on how fast files are updated.
I also thought of comparing Firefox-mozilla-central-nightly-latest to the latest known release (for instance Firefox-mozilla-central-nightly-20171015220106), but if some locales are missing, the JSON will be differents.
@mozbhearsum, would you have an idea on how to fetch the latest buildId of a release?
There was a problem hiding this comment.
We could probably add support for that in Balrog's API. But honestly, just looking at a single platform's en-US buildid is probably good enough.
|
|
||
| @property | ||
| def is_latest_build_update(self): | ||
| return self.rule['mapping'] in self.update_mappings |
There was a problem hiding this comment.
Nit: https://localhost:8015/firefox/beta currently says beta isn't serving the latest build, even though it is. That's because update_mappings are hardcoded to nightly's at https://github.com/mozilla-releng/services/pull/601/files#diff-19ab62af992198ae1d7286a4d5eb5b00R63
We can either not hardcode update_mappings or not allow users to look up states of other channels. I'd suggest the second option. Let's keep the project simple at first. The idea is to show a straightforward page to nightly users, so, they can answer the question: "Is my nightly up to date?"
Speaking of complexity, I just realized we there's a more complex tool already existing: https://mozilla.github.io/delivery-dashboard/#pollbot/firefox/58.0a1. In my opinion, this page has too much information for nightly users who just want to know if they're up to date. What do you think @allan-silva ?
There was a problem hiding this comment.
Yes, this happened because mapping for beta is not configured. I'll keep only nightly updates, but I should check if the rule maps to "Firefox-mozilla-central-nightly-latest" release, right? Or there is another way to determine if the release is the latest?
There was a problem hiding this comment.
You're right, checking if the rule maps to Firefox-mozilla-central-nightly-latest is the way to go.
| <html> | ||
| <head> | ||
| <title> | ||
| PAGE TITLE |
There was a problem hiding this comment.
How about: "Nightly status"?
There was a problem hiding this comment.
Oh yeah, the current HTML is just to you see what informations we have available and to facilitate the review.
| <title> | ||
| PAGE TITLE | ||
| </title> | ||
| <link rel="stylesheet" href="https://maxcdn.bootstrapcdn.com/bootstrap/4.0.0-beta/css/bootstrap.min.css" integrity="sha384-/Y6pD6FV/Vv2HJnA6t+vslU6fwYXjCFtcEpHbNJ0lyAFsXTsjBbfaDjzALeQsN6M" crossorigin="anonymous"> |
There was a problem hiding this comment.
I don't know what's our policy about external CDNs. Using them may be seen as tracking. Maybe @garbas or @mozbhearsum would know.
There was a problem hiding this comment.
@JohanLorenzo good catch.
Lets first get the functionality working. Once we have everything in place we can tighten the security using flask-talisman which is already used in other projects in mozilla-releng/services.
| </tr> | ||
| </thead> | ||
| <tbody> | ||
| {% for locale_name, locale in platform.locales.items() %} |
There was a problem hiding this comment.
I don't think we need to show BuildID per locale, yet. AFAIK, we have usually have issues related to platforms. I don't remember freezing a locale on Nightly. @mozbhearsum may have more insights, though.
There was a problem hiding this comment.
The only case I can think of where buildids could be different by locale is when a locale gets dropped. I think it's okay to ignore this case for now.
|
|
||
| def create_app(config=None): | ||
| app = backend_common.create_app( | ||
| name=releng_channel_status.config.PROJECT_NAME, |
There was a problem hiding this comment.
Bitrot: create_app() now asks for a project_name and a app_name.
|
@JohanLorenzo , I agree! I left query by other channels, because it is easy to get from balrog, clean the code during review is expected. |
|
@JohanLorenzo, @garbas I made more changes, I do not understand yet why mocked requests fails in taskcluster. Some page states bellow.
|
|
@allan-silva It looks good, I'll give it a closer look this evening. |
|
i just rebased this on current master and will start reviewing it shortly. |
|
Closing for while, probably a lot of things changed. |
This is a work in progress. How it is a new project I think we can start the review, since the basic functionality to get information from Balrog is ready.
Currently output is a json fetched from balrog.
TO-DO:
Query by alias:
{base_url}/- fetches the release with default configured alias (firefox-nightly).{base_url}/{alias}- fetches the release with specified alias.Support for query by others products/channels:
{base_url}/{product}/{channel}- fetches the release with specified product and channel