Skip to content
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

Support multiple scope prefixes in bitrisescript #953

Merged
merged 3 commits into from Mar 28, 2024

Conversation

ahal
Copy link
Contributor

@ahal ahal commented Mar 20, 2024

No description provided.

@ahal ahal requested a review from a team March 20, 2024 18:05
@ahal ahal self-assigned this Mar 20, 2024
@ahal ahal marked this pull request as draft March 20, 2024 19:38
@ahal ahal force-pushed the bitrisescript_prefixes branch 4 times, most recently from 429c88a to ccb60e7 Compare March 26, 2024 17:39
It turns out that we need to use consistent scope prefixes as the other
scriptworkers as Taskgraph configures a single prefix across all
scriptworkers. This means that the repo name can appear in the prefix,
which in turn means we'll need to support multiple valid prefixes per
trust domain.

This is how the other scriptworkers work, so it's a good change to make
anyway.
This is actually very verbose and makes the Taskcluster logs very
difficult to read, so only log it on error.
@ahal ahal marked this pull request as ready for review March 26, 2024 17:39
Copy link
Contributor

@hneiva hneiva left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

return [prefix if prefix.endswith(":") else "{}:".format(prefix) for prefix in prefixes]


def extract_common_scope_prefix(config: dict[str, Any], task: dict[str, Any]) -> str:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are these scope resolution functions generic enough to move to something like scriptworker_client.task?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think so, good call! I copy/pasted it from another script and it looks like a bunch of them have something very similar.. Should be a follow-up though.

@ahal ahal merged commit af40990 into mozilla-releng:master Mar 28, 2024
25 checks passed
@ahal ahal deleted the bitrisescript_prefixes branch March 28, 2024 15:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants