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

Fix exists method to support using Requester Pays #46759

Merged
merged 3 commits into from
Feb 23, 2025

Conversation

kylase
Copy link
Contributor

@kylase kylase commented Feb 14, 2025

  • Fix exists method to support user_project for Requester Pays
  • Corrected minor grammatical errors in docstrings

related: #32760


^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in newsfragments.

@boring-cyborg boring-cyborg bot added the provider:google Google (including GCP) related issues label Feb 14, 2025
@kylase kylase marked this pull request as draft February 14, 2025 11:27
@kylase kylase force-pushed the gcs_exists_user_project branch 2 times, most recently from 3ce9ea9 to 65bc5e3 Compare February 16, 2025 09:56
@kylase kylase marked this pull request as ready for review February 16, 2025 10:39
@kylase kylase force-pushed the gcs_exists_user_project branch from 65bc5e3 to bcbfab3 Compare February 17, 2025 01:27
@kylase
Copy link
Contributor Author

kylase commented Feb 17, 2025

@potiuk, how do I get a reviewer or who can I reach out to review this PR? I notice that the google provider has no CODEOWNER.

@VladaZakharova
Copy link
Contributor

Hi! I see that you have added unit test, but can you please provide here a screenshot of successfully executed system test for this logic? Thanks!

@potiuk
Copy link
Member

potiuk commented Feb 23, 2025

Hi! I see that you have added unit test, but can you please provide here a screenshot of successfully executed system test for this logic? Thanks!

I am not sure if people run those :) -> but @kylase -> maybe you can? It's not difficult -> https://github.com/apache/airflow/blob/main/contributing-docs/testing/system_tests.rst

@kylase
Copy link
Contributor Author

kylase commented Feb 23, 2025

I did run the system tests prior to submitting a PR.

Sorry, I have been busy these days, so I wasn't able to respond.

I will provide a screenshot by next week.

@potiuk
Copy link
Member

potiuk commented Feb 23, 2025

No need in this case - we trust you :)

@potiuk potiuk merged commit a215880 into apache:main Feb 23, 2025
61 checks passed
@kylase
Copy link
Contributor Author

kylase commented Feb 25, 2025

@potiuk and @VladaZakharova, sorry for the delay.

image

@kylase kylase deleted the gcs_exists_user_project branch February 25, 2025 07:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
provider:google Google (including GCP) related issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants