You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2022/05/31 10:33:58 UTC
[GitHub] [airflow] gitstart-airflow opened a new pull request, #24039: Fix GCSToGCSOperator cannot copy a single file/folder without copying other files/folders with that prefix
gitstart-airflow opened a new pull request, #24039:
URL: https://github.com/apache/airflow/pull/24039
<!--
Thank you for contributing! Please make sure that your code changes
are covered with tests. And in case of new features or big changes
remember to adjust the documentation.
Feel free to ping committers for the review!
In case of existing issue, reference it using one of the following:
closes: #ISSUE
related: #ISSUE
How to write a good git commit message:
http://chris.beams.io/posts/git-commit/
-->
closes: #22675
related: #22675
---
**^ Add meaningful description above**
This PR fixes the issue whereby source object is treated as prefix which copies objects that are not exact with source object. This PR adds an exact_match flag.
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst#pull-request-guidelines)** for more information.
In case of fundamental code change, Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvements+Proposals)) is needed.
In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
In case of backwards incompatible changes please leave a note in a newsfragement file, named `{pr_number}.significant.rst`, in [newsfragments](https://github.com/apache/airflow/tree/main/newsfragments).
--
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
[GitHub] [airflow] potiuk commented on a diff in pull request #24039: Fix GCSToGCSOperator cannot copy a single file/folder without copying other files/folders with that prefix
Posted by GitBox <gi...@apache.org>.
potiuk commented on code in PR #24039:
URL: https://github.com/apache/airflow/pull/24039#discussion_r890034464
##########
airflow/providers/google/cloud/transfers/gcs_to_gcs.py:
##########
@@ -341,6 +345,8 @@ def _copy_source_without_wildcard(self, hook, prefix):
raise AirflowException(msg)
for source_obj in objects:
+ if self.exact_match and (source_obj != prefix or not source_obj.endswith(prefix)):
+ continue
Review Comment:
Yeah. I see the point. The object store semantics is funny because it resembles filesystem one, but it is in fact object name, so "prefix" can often mean wrong things, for example:
* test_file.zip - might be an object (file) stored
* test_file.zip/another_file.zip - > might be ANOTHER object in something that looks like is in a "test_file.zip" folder.
This is not like that in "real filesystem", you cannot have "file" and "folder" with the same name. That's why "exact-match" is sometimes the only choice as you have no other way to skip the "nested" object..
--
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
[GitHub] [airflow] github-actions[bot] commented on pull request #24039: Fix GCSToGCSOperator cannot copy a single file/folder without copying other files/folders with that prefix
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #24039:
URL: https://github.com/apache/airflow/pull/24039#issuecomment-1147316466
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.
--
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
[GitHub] [airflow] gitstart-airflow commented on a diff in pull request #24039: Fix GCSToGCSOperator cannot copy a single file/folder without copying other files/folders with that prefix
Posted by GitBox <gi...@apache.org>.
gitstart-airflow commented on code in PR #24039:
URL: https://github.com/apache/airflow/pull/24039#discussion_r886705126
##########
airflow/providers/google/cloud/transfers/gcs_to_gcs.py:
##########
@@ -341,6 +345,8 @@ def _copy_source_without_wildcard(self, hook, prefix):
raise AirflowException(msg)
for source_obj in objects:
+ if self.exact_match and (source_obj != prefix or not source_obj.endswith(prefix)):
+ continue
Review Comment:
@uranusjr That's the issue. According to this issue #22675 , treating these objects as prefix copies objects from the source bucket that has no match with the specified ```source_object```. So, checking ```not source_obj.endswith(prefix)``` makes sure we only copy the exact match and not copying all objects with the same prefix as the ```source_objext```
--
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
[GitHub] [airflow] uranusjr commented on a diff in pull request #24039: Fix GCSToGCSOperator cannot copy a single file/folder without copying other files/folders with that prefix
Posted by GitBox <gi...@apache.org>.
uranusjr commented on code in PR #24039:
URL: https://github.com/apache/airflow/pull/24039#discussion_r886177489
##########
airflow/providers/google/cloud/transfers/gcs_to_gcs.py:
##########
@@ -341,6 +345,8 @@ def _copy_source_without_wildcard(self, hook, prefix):
raise AirflowException(msg)
for source_obj in objects:
+ if self.exact_match and (source_obj != prefix or not source_obj.endswith(prefix)):
+ continue
Review Comment:
I’m probably misunderstanding this; shouldn’t `prefix` be compared with `startswith` instead?
--
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
[GitHub] [airflow] potiuk merged pull request #24039: Fix GCSToGCSOperator cannot copy a single file/folder without copying other files/folders with that prefix
Posted by GitBox <gi...@apache.org>.
potiuk merged PR #24039:
URL: https://github.com/apache/airflow/pull/24039
--
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
[GitHub] [airflow] potiuk commented on a diff in pull request #24039: Fix GCSToGCSOperator cannot copy a single file/folder without copying other files/folders with that prefix
Posted by GitBox <gi...@apache.org>.
potiuk commented on code in PR #24039:
URL: https://github.com/apache/airflow/pull/24039#discussion_r890034464
##########
airflow/providers/google/cloud/transfers/gcs_to_gcs.py:
##########
@@ -341,6 +345,8 @@ def _copy_source_without_wildcard(self, hook, prefix):
raise AirflowException(msg)
for source_obj in objects:
+ if self.exact_match and (source_obj != prefix or not source_obj.endswith(prefix)):
+ continue
Review Comment:
Yeah. I see the point. The object store semantics is funny because it resembles filesystem one, but it is in fact object name, so "prefix" can often mean wrong things, for example:
* test_file.zip - might be an object (file) stored
* test_file.zip/another_file.zip - > might be ANOTHER object in something that looks like a "test_file.zip" folder.
This is not like that in "real filesystem", you cannot have "file" and "folder" with the same name. That's why "exact-match" is sometimes the only choice as you have no other way to skip the "nested" object..
--
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