You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "Ajay9704 (via GitHub)" <gi...@apache.org> on 2026/02/02 11:33:59 UTC
[PR] fix to gcs [airflow]
Ajay9704 opened a new pull request, #61347:
URL: https://github.com/apache/airflow/pull/61347
Description
This PR implements consistent GCS destination URI return behavior for GoogleDriveToGCSOperator and GoogleSheetsToGCSOperator
Changes
GoogleDriveToGCSOperator (transfers/gdrive_to_gcs.py)
Added unwrap_single parameter to control return format (default: True)
Returns full GCS URIs in gs://bucket/object format instead of None
Added deprecation warning for future default behavior change
Maintains backward compatibility with existing XCom functionality
GoogleSheetsToGCSOperator (transfers/sheets_to_gcs.py)
Added unwrap_single parameter to control return format (default: True)
Returns full GCS URIs in gs://bucket/object format instead of object names only
Added deprecation warning for future default behavior change
Preserves existing XCom push behavior for destination objects
Implementation Details
Both operators now follow the consistent return convention:
When unwrap_single=True (default): Returns single string for one file, list for multiple files
When unwrap_single=False: Always returns list regardless of file count
Full GCS URI format: All return values include gs:// prefix
Backward compatibility: Existing code continues to work unchanged
Tests
Added comprehensive test coverage for both operators:
test_execute_with_unwrap_single_true - Single file return behavior
test_execute_with_unwrap_single_false - List return behavior
test_execute_with_unwrap_single_default - Default behavior verification
GoogleSheetsToGCSOperator: Additional tests for single vs multiple file scenarios
Related Issues
Part of issue #11323 - Return files destination URIs in all ToGCS operators
Migration Notes
Current behavior (backward compatible):
python
# Returns single URI string when one file, list when multiple
op = GoogleDriveToGCSOperator(...)
result = op.execute(context) # e.g., "gs://bucket/file.txt" or ["gs://bucket/file1.txt", "gs://bucket/file2.txt"]
Future behavior (prepare now):
python
# Explicitly set unwrap_single to avoid future breaking changes
op = GoogleDriveToGCSOperator(unwrap_single=False, ...)
result = op.execute(context) # Always returns list: ["gs://bucket/file.txt"]
Checklist
Added unwrap_single parameter with proper type hints
Implemented deprecation warning for future default change
Updated return values to full GCS URI format (gs://bucket/object)
Maintained backward compatibility with existing XCom behavior
Added comprehensive unit tests for all scenarios
Updated docstrings with new parameter documentation
Verified system test examples remain compatible
Closes #11323 (partial)
Used AI for resolving conflicts
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Return files destination uris in `GoogleDriverToGCSOperator` and `SheetsToGCSOperator` [airflow]
Posted by "yuseok89 (via GitHub)" <gi...@apache.org>.
yuseok89 commented on code in PR #61347:
URL: https://github.com/apache/airflow/pull/61347#discussion_r2854330437
##########
providers/google/src/airflow/providers/google/cloud/transfers/sheets_to_gcs.py:
##########
@@ -128,7 +142,11 @@ def execute(self, context: Context):
for sheet_range in sheet_titles:
data = sheet_hook.get_values(spreadsheet_id=self.spreadsheet_id, range_=sheet_range)
gcs_path_to_file = self._upload_data(gcs_hook, sheet_hook, sheet_range, data)
- destination_array.append(gcs_path_to_file)
+ if self.return_gcs_uris:
+ gcs_uri = f"gs://{self.destination_bucket}/{gcs_path_to_file}"
+ destination_array.append(gcs_uri)
+ else:
+ destination_array.append(gcs_path_to_file)
Review Comment:
For this operator, existing users get object paths from the `destination_objects` XCom key, while the new behavior is exposed via the return value. Since the new feature returns full URIs through the return value, it shouldn’t affect existing users who read from XCom. Is the `return_gcs_uris` flag really necessary?
cc @shahar1
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Return files destination uris in `GoogleDriverToGCSOperator` and `SheetsToGCSOperator` [airflow]
Posted by "Ajay9704 (via GitHub)" <gi...@apache.org>.
Ajay9704 commented on PR #61347:
URL: https://github.com/apache/airflow/pull/61347#issuecomment-3867293733
> > @shahar1 I tried multiple approaches to run the system tests locally, but ran into environment issues each time. The system test DAGs seem to rely on infrastructure that only exists in the CI environment (like the tests_common module, pytest setup, and GCP credentials), so I couldn’t get them working when triggering the DAGs directly from the Airflow UI. I attempted : Importing the system test DAGs directly in the Airflow UI → failed due to missing tests_common. Setting up Breeze → faced connectivity and configuration issues. Trying Docker Compose → same missing-module problems. Setting up the full GCP environment → required extensive configuration and credentials, and I kept running into blockers. I’ve been trying until now, but unfortunately I wasn’t able to run the system tests successfully. If possible, could you please guide me through the correct setup or help with running them? I’ll be happy to follow any steps you suggest.
> > Screencast.From.2026-02-06.17-05-02.mp4
>
> Yes - try to do it using breeze and Airflow's UI, it should be the easiest - please follow these instructions***:
>
> 1. Make sure that you have `gcloud` installed locally on your machine, and you're authenticated to a GCP project
>
> 2. Create the file `files/airflow-breeze-config/init.sh` or use the existing one, and add the following:
>
>
> ```shell
> export AIRFLOW_CONN_GOOGLE_CLOUD_DEFAULT='google-cloud-platform://'
> export GOOGLE_CLOUD_PROJECT='your-gcp-project' # Fill in with GCP project ID
> export SYSTEM_TESTS_GCP_PROJECT='your-gcp-project' # Fill in with GCP project ID
> export SYSTEM_TESTS_ENV_ID='default'
> ```
>
> 3. Copy and paste the system test file to `files/dags`.
>
> 4. Run the test.
>
> 5. When you're done, copy the system test back to the original path - make sure not to commit any unwanted changes (like sensitive data).
>
>
> *** - There are probably better ways to achieve it, but it works for me (and hopefully we could simpify it even further). If there are issues reading the `SYSTEM_TESTS_` variables, just hardcode them in the copied Dag (make sure not to commit them though).
@shahar1 Thanks for reviewing the changes and for the clear guidance.
I attempted to run the system tests using Breeze as instructed, but I am consistently running into an issue where the Airflow services do not start. PostgreSQL initializes successfully, but the webserver, scheduler, triggerer, and API server never come up. I reinstalled Breeze, restarted multiple times, and re-checked my environment, but the problem persists and appears to be related to my local Docker/Breeze setup.
Because of this, I haven't been able to run the system tests locally. Could you advise on how to resolve this Breeze startup issue, or let me know if running system tests in CI would be acceptable for this PR? If unit tests demonstrating the behavior are sufficient, I can proceed accordingly as well.
Any guidance you can provide would be very helpful.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix to gcs [airflow]
Posted by "Ajay9704 (via GitHub)" <gi...@apache.org>.
Ajay9704 commented on PR #61347:
URL: https://github.com/apache/airflow/pull/61347#issuecomment-3839857998
@shahar1 can you please review now , all checks are passing . If there is anything left from my end please let me know
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Return files destination uris in `GoogleDriverToGCSOperator` and `SheetsToGCSOperator` [airflow]
Posted by "Ajay9704 (via GitHub)" <gi...@apache.org>.
Ajay9704 commented on PR #61347:
URL: https://github.com/apache/airflow/pull/61347#issuecomment-3941416595
@yuseok89
Can you please help me with system testing? I’m facing some issues, and your support would be really helpful.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Return files destination uris in `GoogleDriverToGCSOperator` and `SheetsToGCSOperator` [airflow]
Posted by "shahar1 (via GitHub)" <gi...@apache.org>.
shahar1 commented on code in PR #61347:
URL: https://github.com/apache/airflow/pull/61347#discussion_r2855700129
##########
providers/google/src/airflow/providers/google/cloud/transfers/sheets_to_gcs.py:
##########
@@ -128,7 +142,11 @@ def execute(self, context: Context):
for sheet_range in sheet_titles:
data = sheet_hook.get_values(spreadsheet_id=self.spreadsheet_id, range_=sheet_range)
gcs_path_to_file = self._upload_data(gcs_hook, sheet_hook, sheet_range, data)
- destination_array.append(gcs_path_to_file)
+ if self.return_gcs_uris:
+ gcs_uri = f"gs://{self.destination_bucket}/{gcs_path_to_file}"
+ destination_array.append(gcs_uri)
+ else:
+ destination_array.append(gcs_path_to_file)
Review Comment:
Good call, not entirely sure why we need both return value as well as `destination_objects` altogether since they return the same value, even without the proposed changes. I think that the later should be deprecated to avoid redundancy, unless someone couls justify it.
In any case, it's for another PR - for now I prefer that they will be aligned.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Return files destination uris in `GoogleDriverToGCSOperator` and `SheetsToGCSOperator` [airflow]
Posted by "Ajay9704 (via GitHub)" <gi...@apache.org>.
Ajay9704 commented on PR #61347:
URL: https://github.com/apache/airflow/pull/61347#issuecomment-3861725860
@shahar1
I tried multiple approaches to run the system tests locally, but ran into environment issues each time. The system test DAGs seem to rely on infrastructure that only exists in the CI environment (like the tests_common module, pytest setup, and GCP credentials), so I couldn’t get them working when triggering the DAGs directly from the Airflow UI.
I attempted :
Importing the system test DAGs directly in the Airflow UI → failed due to missing tests_common.
Setting up Breeze → faced connectivity and configuration issues.
Trying Docker Compose → same missing-module problems.
Setting up the full GCP environment → required extensive configuration and credentials, and I kept running into blockers.
I’ve been trying until now, but unfortunately I wasn’t able to run the system tests successfully.
If possible, could you please guide me through the correct setup or help with running them?
I’ll be happy to follow any steps you suggest.
https://github.com/user-attachments/assets/8c68f75c-c6c1-4377-9bdc-a6978fff1afc
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Return files destination uris in `GoogleDriverToGCSOperator` and `SheetsToGCSOperator` [airflow]
Posted by "shahar1 (via GitHub)" <gi...@apache.org>.
shahar1 commented on PR #61347:
URL: https://github.com/apache/airflow/pull/61347#issuecomment-3861806328
> @shahar1 I tried multiple approaches to run the system tests locally, but ran into environment issues each time. The system test DAGs seem to rely on infrastructure that only exists in the CI environment (like the tests_common module, pytest setup, and GCP credentials), so I couldn’t get them working when triggering the DAGs directly from the Airflow UI. I attempted : Importing the system test DAGs directly in the Airflow UI → failed due to missing tests_common. Setting up Breeze → faced connectivity and configuration issues. Trying Docker Compose → same missing-module problems. Setting up the full GCP environment → required extensive configuration and credentials, and I kept running into blockers. I’ve been trying until now, but unfortunately I wasn’t able to run the system tests successfully. If possible, could you please guide me through the correct setup or help with running them? I’ll be happy to follow any steps you suggest.
>
> Screencast.From.2026-02-06.17-05-02.mp4
Yes - try to do it using breeze and Airflow's UI, it should be the easiest - please follow these instructions***:
1. Make sure that you have `gcloud` installed locally on your machine, and you're authenticated to a GCP project
2. Create the file `files/airflow-breeze-config/init.sh` or use the existing one, and add the following:
```bash
export AIRFLOW_CONN_GOOGLE_CLOUD_DEFAULT='google-cloud-platform://'
export GOOGLE_CLOUD_PROJECT='your-gcp-project' # Fill in with GCP project ID
export SYSTEM_TESTS_GCP_PROJECT='your-gcp-project' # Fill in with GCP project ID
export SYSTEM_TESTS_ENV_ID='default'
```
3. Copy and paste the system test file to `files/dags`.
4. Run the test.
5. When you're done, copy the system test back to the original path - make sure not to commit any unwanted changes (like sensitive data).
*** - There are probably better ways to achieve it, but it works for me (and it could be simplified even further).
If there are issues reading the `SYSTEM_TESTS_` variables, just hardcode them in the copied Dag (make sure not to commit them though).
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Return files destination uris in `GoogleDriverToGCSOperator` and `SheetsToGCSOperator` [airflow]
Posted by "shahar1 (via GitHub)" <gi...@apache.org>.
shahar1 commented on PR #61347:
URL: https://github.com/apache/airflow/pull/61347#issuecomment-3867417916
> > > @shahar1 I tried multiple approaches to run the system tests locally, but ran into environment issues each time. The system test DAGs seem to rely on infrastructure that only exists in the CI environment (like the tests_common module, pytest setup, and GCP credentials), so I couldn’t get them working when triggering the DAGs directly from the Airflow UI. I attempted : Importing the system test DAGs directly in the Airflow UI → failed due to missing tests_common. Setting up Breeze → faced connectivity and configuration issues. Trying Docker Compose → same missing-module problems. Setting up the full GCP environment → required extensive configuration and credentials, and I kept running into blockers. I’ve been trying until now, but unfortunately I wasn’t able to run the system tests successfully. If possible, could you please guide me through the correct setup or help with running them? I’ll be happy to follow any steps you suggest.
> > > Screencast.From.2026-02-06.17-05-02.mp4
> >
> > Yes - try to do it using breeze and Airflow's UI, it should be the easiest - please follow these instructions***:
> >
> > 1. Make sure that you have `gcloud` installed locally on your machine, and you're authenticated to a GCP project
> >
> > 2. Create the file `files/airflow-breeze-config/init.sh` or use the existing one, and add the following:
> >
> >
> > ```shell
> > export AIRFLOW_CONN_GOOGLE_CLOUD_DEFAULT='google-cloud-platform://'
> > export GOOGLE_CLOUD_PROJECT='your-gcp-project' # Fill in with GCP project ID
> > export SYSTEM_TESTS_GCP_PROJECT='your-gcp-project' # Fill in with GCP project ID
> > export SYSTEM_TESTS_ENV_ID='default'
> > ```
> >
> > 3. Copy and paste the system test file to `files/dags`.
> >
> > 4. Run the test.
> >
> > 5. When you're done, copy the system test back to the original path - make sure not to commit any unwanted changes (like sensitive data).
> >
> >
> > *** - There are probably better ways to achieve it, but it works for me (and hopefully we could simpify it even further). If there are issues reading the `SYSTEM_TESTS_` variables, just hardcode them in the copied Dag (make sure not to commit them though).
>
> @shahar1 Thanks for reviewing the changes and for the clear guidance.
>
> I attempted to run the system tests using Breeze as instructed, but I am consistently running into an issue where the Airflow services do not start. PostgreSQL initializes successfully, but the webserver, scheduler, triggerer, and API server never come up. I reinstalled Breeze, restarted multiple times, and re-checked my environment, but the problem persists and appears to be related to my local Docker/Breeze setup.
>
> Because of this, I haven't been able to run the system tests locally. Could you advise on how to resolve this Breeze startup issue, or let me know if running system tests in CI would be acceptable for this PR? If unit tests demonstrating the behavior are sufficient, I can proceed accordingly as well.
>
> Any guidance you can provide would be very helpful.
What error message/logs do you get? Do you run it using `breeze start-airflow --backend postgres`? (Maybe you could try the default `sqlite` backend as well)
Please try sending a message about it in the Airflow's Slack channel (#airflow-breeze), it will be easier for me to try helping you debugging it there.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Return files destination uris in `GoogleDriverToGCSOperator` and `SheetsToGCSOperator` [airflow]
Posted by "Ajay9704 (via GitHub)" <gi...@apache.org>.
Ajay9704 commented on PR #61347:
URL: https://github.com/apache/airflow/pull/61347#issuecomment-3860705700
> Good job! Please handle my comments. Also, please note that in order to merge it we'll need to run the system tests (same goes for other related PRs you've made). If you're able to do it on your own and attach screenshots it will be the best, otherwise we'll need to wait for Google team or someone else to do it.
@shahar1 Thanks for the review! I'm already working on the changes you mentioned.
I'm also trying to run the system tests locally , earlier Breeze was giving me some connectivity issues, but I'm setting it up again and will update you very soon with the results.
If everything runs fine on my side, I'll attach the screenshots too.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Return files destination uris in `GoogleDriverToGCSOperator` and `SheetsToGCSOperator` [airflow]
Posted by "shahar1 (via GitHub)" <gi...@apache.org>.
shahar1 commented on code in PR #61347:
URL: https://github.com/apache/airflow/pull/61347#discussion_r2772820958
##########
providers/google/src/airflow/providers/google/cloud/transfers/sheets_to_gcs.py:
##########
@@ -128,7 +143,11 @@ def execute(self, context: Context):
for sheet_range in sheet_titles:
data = sheet_hook.get_values(spreadsheet_id=self.spreadsheet_id, range_=sheet_range)
gcs_path_to_file = self._upload_data(gcs_hook, sheet_hook, sheet_range, data)
- destination_array.append(gcs_path_to_file)
+ gcs_uri = f"gs://{self.destination_bucket}/{gcs_path_to_file}"
+ destination_array.append(gcs_uri)
context["ti"].xcom_push(key="destination_objects", value=destination_array)
+
+ if self.unwrap_single:
+ return destination_array[0] if len(destination_array) == 1 else destination_array
return destination_array
Review Comment:
We should handle this operator differently - it already returns a list, but the entities are not in the gs URI format. Therefore:
1. There's no need for the `unwarp_single` flag - the idea is that after aligning the operators to return `list[str]` by default, we could then deprecate the `unwrap_single` where used and then users will access the single results using `lst[0]` only.
2. Instead, we should introduce another flag called `return_gcs_uris` (see comment here: https://github.com/apache/airflow/pull/61048#issuecomment-3831876564) - so we won't break backward-compatibility with current behavior. Default should be as it is today (`False`), and the warning should be that it will later be changed to `True` - you could use the wording from #61048.
##########
providers/google/src/airflow/providers/google/cloud/transfers/gdrive_to_gcs.py:
##########
@@ -100,6 +115,13 @@ def execute(self, context: Context):
) as file:
gdrive_hook.download_file(file_id=file_metadata["id"], file_handle=file)
+ gcs_uri = f"gs://{self.bucket_name}/{self.object_name}"
+ result = [gcs_uri]
+
+ if self.unwrap_single:
+ return result[0]
+ return result
Review Comment:
See my comment for the other operator - we could avoid adding `unwrap_single` here at all, and just `return [gcs_uri]`.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Return files destination uris in `GoogleDriverToGCSOperator` and `SheetsToGCSOperator` [airflow]
Posted by "shahar1 (via GitHub)" <gi...@apache.org>.
shahar1 commented on code in PR #61347:
URL: https://github.com/apache/airflow/pull/61347#discussion_r2772826557
##########
providers/google/src/airflow/providers/google/cloud/transfers/gdrive_to_gcs.py:
##########
@@ -100,6 +115,13 @@ def execute(self, context: Context):
) as file:
gdrive_hook.download_file(file_id=file_metadata["id"], file_handle=file)
+ gcs_uri = f"gs://{self.bucket_name}/{self.object_name}"
+ result = [gcs_uri]
+
+ if self.unwrap_single:
+ return result[0]
+ return result
Review Comment:
See my comment for the other operator - we could avoid adding `unwrap_single` parameter at all, and just `return [gcs_uri]`.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org