-
Notifications
You must be signed in to change notification settings - Fork 16.3k
Allow creation of empty files on GCS #51346
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
Conversation
Passing data="" did not work because it evaluates to False. Made the same change for filename as well, so that filename="" will now give a clearer error when trying to open the file.
|
Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contributors' Guide (https://github.com/apache/airflow/blob/main/contributing-docs/README.rst)
|
kiran2706
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me.
|
Can you please add unit tests ? |
|
No, sorry. That'll turn a trivial five minute fix into a two hour ordeal figuring out your unit test structure, what and where to mock, how to run the tests, and dealing with spurious failures (I think CI already proved this above). I don't have time for that. If you insist, I'd be happy to downgrade this PR into a bug report for someone else to pick up. |
|
@ttencate - I can add those test cases. |
|
@kiran2706 Thank you! |
|
Thank you @kiran2706 ! I'll go ahead and close this one, no need to keep it open until yours is merged. |
Passing
data=""did not work because it evaluates toFalse.Made the same change for
filenameas well, so thatfilename=""will now give a clearer error when trying to open the file.