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/06/09 17:39:22 UTC
[GitHub] [airflow] Lavedonio opened a new pull request, #24353: GCSDeleteObjectsOperator empty prefix bug fix
Lavedonio opened a new pull request, #24353:
URL: https://github.com/apache/airflow/pull/24353
<!--
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: #24352
How to write a good git commit message:
http://chris.beams.io/posts/git-commit/
-->
This PR solves the Issue #24352 for a bug found in GCSDeleteObjectsOperator, where an error was thrown if the parameter `prefix` was set to an empty string, which should be a valid input.
---
**^ Add meaningful description above**
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] pankajastro commented on a diff in pull request #24353: GCSDeleteObjectsOperator empty prefix bug fix
Posted by GitBox <gi...@apache.org>.
pankajastro commented on code in PR #24353:
URL: https://github.com/apache/airflow/pull/24353#discussion_r894803146
##########
airflow/providers/google/cloud/operators/gcs.py:
##########
@@ -305,8 +305,10 @@ def __init__(
self.delegate_to = delegate_to
self.impersonation_chain = impersonation_chain
- if not objects and not prefix:
- raise ValueError("Either object or prefix should be set. Both are None")
+ if objects is None and prefix is None:
Review Comment:
what about replacing this line with the below snippet?
```
if (objects or prefix) is None:
```
--
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 #24353: GCSDeleteObjectsOperator empty prefix bug fix
Posted by GitBox <gi...@apache.org>.
potiuk merged PR #24353:
URL: https://github.com/apache/airflow/pull/24353
--
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 #24353: GCSDeleteObjectsOperator empty prefix bug fix
Posted by GitBox <gi...@apache.org>.
potiuk commented on code in PR #24353:
URL: https://github.com/apache/airflow/pull/24353#discussion_r895073478
##########
airflow/providers/google/cloud/operators/gcs.py:
##########
@@ -305,8 +305,10 @@ def __init__(
self.delegate_to = delegate_to
self.impersonation_chain = impersonation_chain
- if not objects and not prefix:
- raise ValueError("Either object or prefix should be set. Both are None")
+ if objects is None and prefix is None:
Review Comment:
I think the current way is easier and more explicit.
--
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] boring-cyborg[bot] commented on pull request #24353: GCSDeleteObjectsOperator empty prefix bug fix
Posted by GitBox <gi...@apache.org>.
boring-cyborg[bot] commented on PR #24353:
URL: https://github.com/apache/airflow/pull/24353#issuecomment-1151413096
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)
Here are some useful points:
- Pay attention to the quality of your code (flake8, mypy and type annotations). Our [pre-commits]( https://github.com/apache/airflow/blob/main/STATIC_CODE_CHECKS.rst#prerequisites-for-pre-commit-hooks) will help you with that.
- In case of a new feature add useful documentation (in docstrings or in `docs/` directory). Adding a new operator? Check this short [guide](https://github.com/apache/airflow/blob/main/docs/apache-airflow/howto/custom-operator.rst) Consider adding an example DAG that shows how users should use it.
- Consider using [Breeze environment](https://github.com/apache/airflow/blob/main/BREEZE.rst) for testing locally, itโs a heavy docker but it ships with a working Airflow and a lot of integrations.
- Be patient and persistent. It might take some time to get a review or get the final approval from Committers.
- Please follow [ASF Code of Conduct](https://www.apache.org/foundation/policies/conduct) for all communication including (but not limited to) comments on Pull Requests, Mailing list and Slack.
- Be sure to read the [Airflow Coding style]( https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst#coding-style-and-best-practices).
Apache Airflow is a community-driven project and together we are making it better ๐.
In case of doubts contact the developers at:
Mailing List: dev@airflow.apache.org
Slack: https://s.apache.org/airflow-slack
--
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 pull request #24353: GCSDeleteObjectsOperator empty prefix bug fix
Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #24353:
URL: https://github.com/apache/airflow/pull/24353#issuecomment-1159819056
@Lavedonio - just pinging when you'v done what you are asked for. Pushing an updated commit does not automatically mean information "I applied the fixes" - the best way to inform the maintainers is to ping them and mention it specifically that "you fixed it". You should be aware we have sometimes 30-50 conversation to look at a day - where you have just one PR. Watch my presentation here to understand more https://www.youtube.com/watch?v=G6VjYvKr2wQ
--
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] Lavedonio commented on pull request #24353: GCSDeleteObjectsOperator empty prefix bug fix
Posted by GitBox <gi...@apache.org>.
Lavedonio commented on PR #24353:
URL: https://github.com/apache/airflow/pull/24353#issuecomment-1158035420
Is anything else needed on my side?
--
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 pull request #24353: GCSDeleteObjectsOperator empty prefix bug fix
Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #24353:
URL: https://github.com/apache/airflow/pull/24353#issuecomment-1154847161
Static checkes are failing
--
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] Lavedonio commented on pull request #24353: GCSDeleteObjectsOperator empty prefix bug fix
Posted by GitBox <gi...@apache.org>.
Lavedonio commented on PR #24353:
URL: https://github.com/apache/airflow/pull/24353#issuecomment-1155142555
> Static checkes are failing
Just saw it right now. I can check that later today.
--
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] Lavedonio commented on pull request #24353: GCSDeleteObjectsOperator empty prefix bug fix
Posted by GitBox <gi...@apache.org>.
Lavedonio commented on PR #24353:
URL: https://github.com/apache/airflow/pull/24353#issuecomment-1159822210
@potiuk thanks for pointing that out. This is the first contribution that I've made to an open-source project, so I wasn't quite aware of how this final interaction worked. I'll make sure to watch your presentation later, thanks for the resource!
--
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] Lavedonio commented on pull request #24353: GCSDeleteObjectsOperator empty prefix bug fix
Posted by GitBox <gi...@apache.org>.
Lavedonio commented on PR #24353:
URL: https://github.com/apache/airflow/pull/24353#issuecomment-1155990168
All checks passed.
@turbaszek I think your review might also be needed in order to merge.
--
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 #24353: GCSDeleteObjectsOperator empty prefix bug fix
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #24353:
URL: https://github.com/apache/airflow/pull/24353#issuecomment-1153007382
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] eladkal commented on pull request #24353: GCSDeleteObjectsOperator empty prefix bug fix
Posted by GitBox <gi...@apache.org>.
eladkal commented on PR #24353:
URL: https://github.com/apache/airflow/pull/24353#issuecomment-1156005317
@Lavedonio can you add test case with empty string to verify it works as expected and to avoid regression?
--
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] boring-cyborg[bot] commented on pull request #24353: GCSDeleteObjectsOperator empty prefix bug fix
Posted by GitBox <gi...@apache.org>.
boring-cyborg[bot] commented on PR #24353:
URL: https://github.com/apache/airflow/pull/24353#issuecomment-1159818674
Awesome work, congrats on your first merged pull request!
--
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