Add a table to keep product channel versions and APIs to update them #1465
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
gabrielBusta
changed the title
gabrielBusta
commented
May 10, 2024
gabrielBusta
force-pushed
the
1462
branch
4 times, most recently
from
4b3649d to
933ff85
Compare
gabrielBusta
changed the title
|
To grant shipitscript scopes to call the API to update versions we'd need a patch in ci-configuration: diff --git a/clients.yml b/clients.yml
--- a/clients.yml
+++ b/clients.yml
@@ -862,6 +862,8 @@ project/releng/scriptworker/v2/shipit/de
- project:releng:services/shipit_api/add_release/devedition
- project:releng:services/shipit_api/add_release/fennec
- project:releng:services/shipit_api/add_release/firefox
+ - project:releng:services/shipit_api/update_product_channel_version/firefox
- project:releng:services/shipit_api/dev/add_release/devedition
- project:releng:services/shipit_api/dev/add_release/firefox
- project:releng:services/shipit_api/dev/schedule_phase/devedition/ship_devedition
@@ -897,6 +899,8 @@ project/releng/scriptworker/v2/shipit/pr
- project:releng:services/shipit_api/add_release/devedition
- project:releng:services/shipit_api/add_release/fennec
- project:releng:services/shipit_api/add_release/firefox
+ - project:releng:services/shipit_api/update_product_channel_version/firefox
- project:releng:services/shipit_api/dev/add_release/devedition
- project:releng:services/shipit_api/dev/add_release/firefox
- project:releng:services/shipit_api/dev/schedule_phase/devedition/ship_devedition
@@ -918,6 +922,8 @@ project/releng/scriptworker/v2/shipit/pr
- project:releng:services/shipit_api/add_release/devedition
- project:releng:services/shipit_api/add_release/fennec
- project:releng:services/shipit_api/add_release/firefox
+ - project:releng:services/shipit_api/update_product_channel_version/firefox
- project:releng:services/shipit_api/production/add_release/devedition
- project:releng:services/shipit_api/production/add_release/firefox
- project:releng:services/shipit_api/production/schedule_phase/devedition/ship_devedition |
gabrielBusta
force-pushed
the
1462
branch
3 times, most recently
from
2a7d431 to
d9e1baa
Compare
gabrielBusta
force-pushed
the
1462
branch
4 times, most recently
from
48e2b2a to
1b8909e
Compare
|
This needs to be followed by a commit 7fef0f6 to use the Firefox Nightly version stored in the database when rebuilding product details. The reason this has to land first is that I need to seed the initial Firefox Nightly version in the database. Otherwise product details will fail because the new table is empty. |
gabrielBusta
commented
May 31, 2024
| @@ -99,7 +99,8 @@ | |||
| AUTH0_AUTH_SCOPES = assign_ldap_groups_to_scopes() | |||
|
|
|||
| # other scopes | |||
| AUTH0_AUTH_SCOPES.update({"rebuild_product_details": LDAP_GROUPS["firefox-signoff"], "update_release_status": []}) | |||
| AUTH0_AUTH_SCOPES.update({"rebuild_product_details": LDAP_GROUPS["firefox-signoff"], "update_release_status": [], "create_product_channel_version/firefox": []}) | |||
There was a problem hiding this comment.
Another follow-up: We will need to grant releng scopes to seed the initial Thunderbird version once the Thunderbird bits are implemented.
Suggested change
| AUTH0_AUTH_SCOPES.update({"rebuild_product_details": LDAP_GROUPS["firefox-signoff"], "update_release_status": [], "create_product_channel_version/firefox": []}) | |
| AUTH0_AUTH_SCOPES.update( | |
| { | |
| "rebuild_product_details": LDAP_GROUPS["firefox-signoff"], | |
| "update_release_status": [], | |
| "create_product_channel_version/firefox": [], | |
| "create_product_channel_version/thunderbird": [], | |
| } | |
| ) |
|
I ran a staging Nightly to try out this APIs in dev. |
hneiva
approved these changes
Jun 7, 2024
| def get_product_channel_version(product, channel): | ||
| version = Version.query.filter_by(product_name=product, product_channel=channel).first() | ||
| if version: | ||
| return version.current_version |
There was a problem hiding this comment.
nit: All other functions here seem to return a tuple with (data, exitcode) should we follow the same pattern here?
| required_permission = f"{SCOPE_PREFIX}/update_product_channel_version/{product}" | ||
| if not current_user.has_permissions(required_permission): | ||
| user_permissions = ", ".join(current_user.get_permissions()) | ||
| abort(401, f"required permission: {required_permission}, user permissions: {user_permissions}") |
There was a problem hiding this comment.
nit: What's the difference between aborting and returning an error?
Also, seems like (data, exitcode) should be the pattern used?
| required_permission = f"{SCOPE_PREFIX}/create_product_channel_version/{product}" | ||
| if not current_user.has_permissions(required_permission): | ||
| user_permissions = ", ".join(current_user.get_permissions()) | ||
| abort(401, f"required permission: {required_permission}, user permissions: {user_permissions}") |
| user_permissions = ", ".join(current_user.get_permissions()) | ||
| abort(401, f"required permission: {required_permission}, user permissions: {user_permissions}") | ||
| existing_version = Version.query.filter_by(product_name=product, product_channel=channel).first() | ||
| if existing_version: |
There was a problem hiding this comment.
nit: could improve readability here:
if Version.query.filter_by(product_name=product, product_channel=channel).first():
return {"error": f"A {product} {channel} version already exists."}, 409
# code here without else
Thanks, I will address on a follow-up |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is an API that lets us store the current versions of FIREFOX_NIGHTLY, instead of hard-coding it, so we can update this data using an API from a
shipitscripttask. This is part of the solution to #1462