Copilot commented on code in PR #56324:
URL: https://github.com/apache/airflow/pull/56324#discussion_r3066475480
##########
providers/google/src/airflow/providers/google/common/hooks/base_google.py:
##########
@@ -342,6 +381,19 @@ def get_credentials_and_project_id(self) ->
tuple[Credentials, str | None]:
idp_extra_params_dict=idp_extra_params_dict,
)
+ # Apply quota project before caching credentials
+ quota_project = self.quota_project_id or
self._get_field("quota_project_id")
+ if quota_project and not is_anonymous:
+ self._validate_quota_project(quota_project)
+ if not hasattr(credentials, "with_quota_project"):
+ raise ValueError(
+ f"Credentials of type {type(credentials).__name__} do not
support "
+ "quota project configuration. Please use a different
authentication method "
+ "or remove the quota_project_id setting."
+ )
Review Comment:
In Airflow hooks, user-facing configuration errors are usually raised as
`AirflowException` so they surface cleanly in task logs/UI and are handled
consistently across providers. Consider raising `AirflowException` here (and in
quota project validation) instead of `ValueError`.
##########
providers/google/src/airflow/providers/google/common/hooks/base_google.py:
##########
@@ -342,6 +381,19 @@ def get_credentials_and_project_id(self) ->
tuple[Credentials, str | None]:
idp_extra_params_dict=idp_extra_params_dict,
)
+ # Apply quota project before caching credentials
+ quota_project = self.quota_project_id or
self._get_field("quota_project_id")
+ if quota_project and not is_anonymous:
+ self._validate_quota_project(quota_project)
+ if not hasattr(credentials, "with_quota_project"):
Review Comment:
`hasattr(credentials, \"with_quota_project\")` only verifies the attribute
exists, not that it’s callable. If `with_quota_project` exists but is
`None`/non-callable, the subsequent call will fail with a less clear exception.
Prefer checking `callable(getattr(credentials, \"with_quota_project\", None))`
and raising the intended configuration error when it’s not callable.
```suggestion
if not callable(getattr(credentials, "with_quota_project",
None)):
```
##########
providers/google/docs/connections/gcp.rst:
##########
@@ -309,3 +325,74 @@ Note that as domain-wide delegation is currently supported
by most of the Google
* All of Google Cloud operators and hooks.
* Firebase hooks.
* All transfer operators that involve Google cloud in different providers, for
example:
:class:`airflow.providers.amazon.aws.transfers.gcs_to_s3.GCSToS3Operator`.
+
+
+Quota Project Support
+---------------------
+
+Airflow's Google Cloud providers support specifying a "quota project" (a
billing project) for
+API calls. That lets API usage be billed to a different Google Cloud project
than the one that
+owns the service account. This is useful for organizations that share service
accounts but
+centralize billing in specific projects.
+
+Usage
+~~~~~
+
+There are two ways to set a quota project in Airflow:
+
+- Via connection extras (recommended for environment-wide defaults).
+- Directly on operators or hooks (recommended when a single task must bill to
a different project).
+
+Connection extras
+^^^^^^^^^^^^^^^^^
+
+Add the quota project ID to the Google Cloud connection extras. For example:
+
+.. code-block:: json
+
+ {
+ "quota_project_id": "your-billing-project-id"
+ }
+
+You can set this via the Airflow UI, the Connections REST API, or an
environment variable, for
+example:
+
+.. code-block:: bash
+
+ export AIRFLOW_CONN_GOOGLE_CLOUD_DEFAULT='{
+ "conn_type": "google-cloud-platform",
+ "extra": {
+ "quota_project_id": "your-billing-project-id"
+ }
+ }'
Review Comment:
The `AIRFLOW_CONN_<CONN_ID>` environment variable is typically parsed as a
connection URI, not as a JSON object. This example is likely to be
rejected/misparsed by Airflow’s env-var connection backend. Consider updating
the docs to use a proper connection URI example (query params/extra) or
explicitly point users to `airflow connections add --conn-json` / Connections
REST API for JSON payloads.
```suggestion
You can set this via the Airflow UI or the Connections REST API using the
extras JSON shown
above. If you are using an environment variable, provide the connection as a
URI, for example:
.. code-block:: bash
export
AIRFLOW_CONN_GOOGLE_CLOUD_DEFAULT='google-cloud-platform://?extra__google_cloud_platform__quota_project_id=your-billing-project-id'
```
##########
providers/google/src/airflow/providers/google/common/hooks/base_google.py:
##########
@@ -123,6 +124,24 @@ def is_refresh_credentials_exception(exception: Exception)
-> bool:
return False
+def is_valid_gcp_project_id(project_id: str) -> bool:
+ """
+ Validate a Google Cloud Project ID format.
+
+ A valid project ID must:
+
+ - Be 6 to 30 characters long
+ - Start with a lowercase letter
+ - Contain only lowercase letters, digits, and hyphens
+ - Not end with a hyphen
+
+ :param project_id: The project ID string to validate.
+ :return: True if the project ID is valid, False otherwise.
+ """
+ pattern = re.compile(r"^[a-z][a-z0-9\-]{4,28}[a-z0-9]$")
+ return bool(pattern.match(project_id))
Review Comment:
This recompiles the regex on every validation call. Since the pattern is
constant, consider compiling it once at module scope (e.g., a
`_GCP_PROJECT_ID_RE`) and reusing it. This also makes the validation logic
easier to test and maintain.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
