fix(dryrun): Run authenticated dryrun queries in mozdata rather than the target project - #9845
Conversation
…n the target project.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment has been minimized.
This comment has been minimized.
Since deploys for targets might use custom service accounts that don't have permission to run queries in `mozdata`.
Integration report
|
|
@claude what do you think now? |
There was a problem hiding this comment.
This PR splits the dry_run config's default_projects into cloud_function_billing_projects (random pick, cloud-function path) and a new default_billing_project: mozdata for the authenticated path, stops mutating self.client.project in favour of client.query(..., project=...), and — in the follow-up commit — threads an explicit billing_project through the three deploy-path dry runs.
Re: the earlier --target concern, the follow-up commit resolves it. I traced the flow: prepare_target_directory builds sql/<effective_project>/... where effective_project = target.project_id or source_project (bigquery_etl/util/target.py:1130-1138), and project_id is a required field on Target (bigquery_etl/util/target.py:101), so the project component of the copied artifact path is always a real GCP project the sandbox SA can create jobs in. deploy is also the only command that forces use_cloud_function=False for targets (bigquery_etl/cli/deploy.py:497-498), and all three authenticated dry runs reachable from it now pass the artifact's project. The other Schema.from_query_file / Schema.for_table call sites in the deploy flow (bigquery_etl/cli/deploy.py:813, bigquery_etl/cli/deploy.py:834, bigquery_etl/view/__init__.py:216, bigquery_etl/util/target.py:641) all leave use_cloud_function at its True default, so they're unaffected. Client.query treats project=None as "use the client's project", so the None branches are safe, and the default_projects rename has no remaining references in the repo.
Remaining comments are minor and inline.
| self.dataset = dataset | ||
| self.table = table | ||
| self.billing_project = billing_project | ||
| # if using cloud function and billing project isn't set, randomly select project to use |
There was a problem hiding this comment.
nitpick: the comment now only describes the use_cloud_function half of the branch it sits above. Suggest something like # if the billing project isn't set, use the configured default (a random cloud function project, or default_billing_project for authenticated dry runs).
| tmp_dataset="tmp", # Default dataset for temporary tables during schema updates | ||
| tmp_tables={}, | ||
| use_cloud_function=options["use_cloud_function"], | ||
| billing_project=(project_id if not options["use_cloud_function"] else None), |
There was a problem hiding this comment.
suggestion: this (<artifact project> if not use_cloud_function else None) expression is now duplicated at three deploy call sites — here, bigquery_etl/cli/deploy.py:1280, and bigquery_etl/deploy.py:107 — and each derives the project differently (extract_from_query_path here vs. file_path.parent.parent.parent.name in the other two).
Since correct --target behaviour now depends on every authenticated dry run in the deploy path carrying this argument, a future deploy-path dry run that omits it will silently bill to mozdata and 403 for sandbox service accounts. Consider a single helper (e.g. dryrun_billing_project(project, use_cloud_function) in bigquery_etl/dryrun.py) that the three sites call, so the policy lives in one place.
3 participants
Description
Because not everyone has permission to run queries in the target projects like
moz-fx-data-shared-prod.Related Tickets & Documents
bqetl query validatesupports billing-project #7598Reviewer, please follow this checklist