-
Notifications
You must be signed in to change notification settings - Fork 13.7k
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
Optionally raise an error if source file does not exist in GCSToGCSOperator #21391
Conversation
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 Contribution Guide (https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst)
|
Could you please add a unit test for it ? You can use the existing tests as a base. |
msg = ( | ||
f'{prefix} does not exist in bucket {self.source_bucket}' | ||
) | ||
self.log.warning(msg) |
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.
maybe move the log inside the if, so current users do not see logs with WARNING level
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.
you're right! I've just modified it
@potiuk I've added a unit test |
The PR is likely OK to be merged with just subset of tests for default Python and Database versions without running the full matrix of tests, because it does not modify the core of Airflow. If the committers decide that the full tests matrix is needed, they will add the label 'full tests needed'. Then you should rebase to the latest main or amend the last commit of the PR, and push it with --force-with-lease. |
Any chance to fix the static check @davidpr91 (shortly as I am preparing RC2 release tonight)? |
:param source_object_required: When source_object_required is True, if you want to copy / move a specific blob | ||
and it doesn't exist, an exception is raised and the task is marked as failed. | ||
This parameter doesn't have any effect when the source_object that you pass is a folder or pattern. |
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.
can we simplify it? When ___ is true, if ....
is a sentence that is hard to follow.
You need to fix the sentence any way as it fails the static checks (line to long) so please try to simplify it.
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.
I've simplified the description. Thanks for the feedback!
@potiuk static checks should be fixed now |
Hello. I see there is a failing check, but I'm not sure how this is related to my change. Could you help me? Thank you! |
d67a8c6
to
18864ed
Compare
@potiuk It worked. Thank you! |
Not entirely it seems:). Let me close/reopen to rebuild. I think we need to add a trrigger for another even in our CI :) |
It worked indeed :) |
Awesome work, congrats on your first merged pull request! |
Right now when using GCSToGCSOperator to copy a file from one bucket to another, if the source file does not exist, nothing happens and the task is considered successful. This could be good for some use cases, for example, when you want to copy all the files from a directory or that match a specific pattern.
But for some other cases, like when you only want to copy one specific blob, it might be useful to raise an exception if the source file can't be found. Otherwise, the task would be failing silently.
This PR adds the flag "source_object_required" to GCSToGCSOperator to enable this feature. By default, for backward compatibility, the value set to False. If we pass set the flag as True, then, if the source_objects are blobs (not folders or patterns) and they don't exist, the task will fail.
closes: #21388
^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code change, 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 UPDATING.md.