-
Notifications
You must be signed in to change notification settings - Fork 26
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
product-details: don't ask hg.mozilla.org for data we already have (bug 1936479) #1586
Conversation
…ug 1936479) The l10n data for a shipped release is immutable, so there's no point gathering it over and over. This means get_primary_builds() needs to look at old_product_details as well. Fixes mozilla-releng#1582
Output from local testing:
|
@@ -211,11 +211,19 @@ async def fetch_l10n_data( | |||
session: aiohttp.ClientSession, release: shipit_api.common.models.Release, raise_on_failure: bool, use_cache: bool = True | |||
) -> typing.Tuple[shipit_api.common.models.Release, typing.Optional[ReleaseL10ns]]: | |||
# Fenix and some thunderbird on the betas don't have l10n in the repository | |||
# XXX: for some reason we didn't generate l10n for devedition in old_product_details |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Obviously this is not a new thing, but we should probably get this on file at the very least. I imagine we could either backfill (and start generating these in the future), or possibly just use the Firefox-xxxx.json files for DevEdition?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Filed as #1588.
locales = ["en-US"] | ||
if release in releases_l10n: | ||
locales += releases_l10n[release].keys() | ||
elif f"1.0/l10n/{release.name}.json" in old_product_details: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's annoying that there's two places that we have to consult old_product_details
:(. In some ideal world, releases_l10n
would have combined the two data sources already, but I don't think that refactor makes sense in the immediate term. Maybe file for a future improvement, if you agree?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Filed as #1587.
It looks like this broke in prod for DevEdition :( https://mozilla.sentry.io/issues/6138643868/?alert_rule_id=14011861&alert_type=issue¬ification_uuid=26c35ba9-1f19-46fc-98b1-28e22632153a&project=6262522&referrer=slack ``` ClickException: Too few firefox primary builds for FIREFOX_DEVEDITION File "aioamqp/protocol.py", line 312, in run await self.dispatch_frame() File "aioamqp/protocol.py", line 268, in dispatch_frame await channel.dispatch_frame(frame) File "aioamqp/channel.py", line 114, in dispatch_frame await methods[frame.name](frame) File "aioamqp/channel.py", line 546, in basic_deliver await callback(self, body, envelope, properties) File "/app/src/src/shipit_api/admin/worker.py", line 27, in rebuild_product_details_async await rebuild(flask.current_app.db.session, app_channel, git_repo_url, folder_in_repo, breakpoint_version) File "/app/src/src/shipit_api/admin/product_details.py", line 1266, in rebuild sanity_checks(product_details) File "/app/src/src/shipit_api/admin/product_details.py", line 1091, in sanity_checks sanity_check_firefox_builds( File "/app/src/src/shipit_api/admin/product_details.py", line 1063, in sanity_check_firefox_builds raise click.ClickException(f"Too few firefox primary builds for {version_key}") ```
It looks like this broke in prod for DevEdition :(
I'm reverting this in #1590 for now. |
It looks like this broke in prod for DevEdition :( https://mozilla.sentry.io/issues/6138643868/?alert_rule_id=14011861&alert_type=issue¬ification_uuid=26c35ba9-1f19-46fc-98b1-28e22632153a&project=6262522&referrer=slack ``` ClickException: Too few firefox primary builds for FIREFOX_DEVEDITION File "aioamqp/protocol.py", line 312, in run await self.dispatch_frame() File "aioamqp/protocol.py", line 268, in dispatch_frame await channel.dispatch_frame(frame) File "aioamqp/channel.py", line 114, in dispatch_frame await methods[frame.name](frame) File "aioamqp/channel.py", line 546, in basic_deliver await callback(self, body, envelope, properties) File "/app/src/src/shipit_api/admin/worker.py", line 27, in rebuild_product_details_async await rebuild(flask.current_app.db.session, app_channel, git_repo_url, folder_in_repo, breakpoint_version) File "/app/src/src/shipit_api/admin/product_details.py", line 1266, in rebuild sanity_checks(product_details) File "/app/src/src/shipit_api/admin/product_details.py", line 1091, in sanity_checks sanity_check_firefox_builds( File "/app/src/src/shipit_api/admin/product_details.py", line 1063, in sanity_check_firefox_builds raise click.ClickException(f"Too few firefox primary builds for {version_key}") ```
I can reproduce #1586 (comment), if there's a devedition release without corresponding firefox release, as was the case for ~20 minutes on Friday (Devedition-134.0b10-build1 was marked as shipped at 16:40 UTC, Firefox-134.0b10-build1 at 16:59). |
That sounds reasonable to me; the check seems to have outlived whatever value it used to provide. |
Looking at this some more... sanity_check_firefox_builds comes from mozilla/release-services#1987. devedition was then added to get_primary_builds in mozilla/release-services#2111. So it seems plausible that bedrock needs the FIREFOX_DEVEDITION version to show up in firefox_primary_builds.json. |
The l10n data for a shipped release is immutable, so there's no point gathering it over and over.
This means get_primary_builds() needs to look at old_product_details as well.
Fixes #1582