You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "Jayyadav1903 (via GitHub)" <gi...@apache.org> on 2026/01/17 19:08:11 UTC
[PR] Refactor Bigtable Delete Instance Operator to remove redundant try/excep… [airflow]
Jayyadav1903 opened a new pull request, #60727:
URL: https://github.com/apache/airflow/pull/60727
This PR removes redundant `try/except` blocks in `BigtableDeleteInstanceOperator` in `airflow/providers/google/cloud/operators/bigtable.py`.
The underlying `BigtableHook.delete_instance` method already handles the case where the instance does not exist by logging a warning. Therefore, catching `NotFound` in the operator was unnecessary and duplicated logic.
I have also:
- Updated the unit tests in `providers/google/tests/unit/google/cloud/operators/test_bigtable.py` to remove tests that specifically enforced this redundant exception handling.
- Corrected the docstring in `airflow/providers/google/cloud/hooks/bigtable.py` which incorrectly stated that `delete_instance` raises `NotFound`.
Generated-by: Antigravity (Google DeepMind)
--
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
Re: [PR] Refactor Bigtable Delete Instance Operator to remove redundant try/excep… [airflow]
Posted by "Jayyadav1903 (via GitHub)" <gi...@apache.org>.
Jayyadav1903 commented on PR #60727:
URL: https://github.com/apache/airflow/pull/60727#issuecomment-3768166043
This was my very first PR as I am a rookie in open source and would try to resolve all the issues at the earliest.
And I would also like to apologize for my impolite actions and would keep in mind not to repeat them again.
--
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
Re: [PR] Refactor Bigtable Delete Instance Operator to remove redundant try/excep… [airflow]
Posted by "Jayyadav1903 (via GitHub)" <gi...@apache.org>.
Jayyadav1903 commented on PR #60727:
URL: https://github.com/apache/airflow/pull/60727#issuecomment-3778073779
I'll surely do that.
--
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
Re: [PR] Refactor Bigtable Delete Instance Operator to remove redundant try/excep… [airflow]
Posted by "sjyangkevin (via GitHub)" <gi...@apache.org>.
sjyangkevin commented on code in PR #60727:
URL: https://github.com/apache/airflow/pull/60727#discussion_r2701998515
##########
PR_DESCRIPTION.md:
##########
Review Comment:
This looks like a file generated by the AI tool, we may not need this file here as it is the description for the PR.
##########
providers/google/src/airflow/providers/google/cloud/operators/bigtable.py:
##########
@@ -331,17 +331,7 @@ def execute(self, context: Context) -> None:
gcp_conn_id=self.gcp_conn_id,
impersonation_chain=self.impersonation_chain,
)
- try:
- hook.delete_instance(project_id=self.project_id, instance_id=self.instance_id)
- except google.api_core.exceptions.NotFound:
- self.log.info(
- "The instance '%s' does not exist in project '%s'. Consider it as deleted",
- self.instance_id,
- self.project_id,
- )
- except google.api_core.exceptions.GoogleAPICallError as e:
- self.log.error("An error occurred. Exiting.")
- raise e
+ hook.delete_instance(project_id=self.project_id, instance_id=self.instance_id)
Review Comment:
another case in the error handling `except google.api_core.exceptions.GoogleAPICallError as e` is also removed. In this case, an error need to be raised instead of showing a logging. It looks different from the PR description that only `NotFound` looks redundant.
--
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
Re: [PR] Refactor Bigtable Delete Instance Operator to remove redundant try/excep… [airflow]
Posted by "jroachgolf84 (via GitHub)" <gi...@apache.org>.
jroachgolf84 commented on code in PR #60727:
URL: https://github.com/apache/airflow/pull/60727#discussion_r2702058699
##########
PR_DESCRIPTION.md:
##########
Review Comment:
This file is not needed and should be removed.
--
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
Re: [PR] Refactor Bigtable Delete Instance Operator to remove redundant try/excep… [airflow]
Posted by "Jayyadav1903 (via GitHub)" <gi...@apache.org>.
Jayyadav1903 closed pull request #60727: Refactor Bigtable Delete Instance Operator to remove redundant try/excep…
URL: https://github.com/apache/airflow/pull/60727
--
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
Re: [PR] Refactor Bigtable Delete Instance Operator to remove redundant try/excep… [airflow]
Posted by "boring-cyborg[bot] (via GitHub)" <gi...@apache.org>.
boring-cyborg[bot] commented on PR #60727:
URL: https://github.com/apache/airflow/pull/60727#issuecomment-3764218443
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)
Here are some useful points:
- Pay attention to the quality of your code (ruff, mypy and type annotations). Our [prek-hooks]( https://github.com/apache/airflow/blob/main/contributing-docs/08_static_code_checks.rst#prerequisites-for-prek-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/airflow-core/docs/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/dev/breeze/doc/README.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-docs/05_pull_requests.rst#coding-style-and-best-practices).
- Always keep your Pull Requests rebased, otherwise your build might fail due to changes not related to your commits.
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
Re: [PR] Refactor Bigtable Delete Instance Operator to remove redundant try/excep… [airflow]
Posted by "shahar1 (via GitHub)" <gi...@apache.org>.
shahar1 commented on code in PR #60727:
URL: https://github.com/apache/airflow/pull/60727#discussion_r2702757737
##########
providers/google/src/airflow/providers/google/cloud/operators/bigtable.py:
##########
@@ -331,17 +331,7 @@ def execute(self, context: Context) -> None:
gcp_conn_id=self.gcp_conn_id,
impersonation_chain=self.impersonation_chain,
)
- try:
- hook.delete_instance(project_id=self.project_id, instance_id=self.instance_id)
- except google.api_core.exceptions.NotFound:
- self.log.info(
- "The instance '%s' does not exist in project '%s'. Consider it as deleted",
- self.instance_id,
- self.project_id,
- )
- except google.api_core.exceptions.GoogleAPICallError as e:
- self.log.error("An error occurred. Exiting.")
- raise e
+ hook.delete_instance(project_id=self.project_id, instance_id=self.instance_id)
Review Comment:
If such exception occurs, it will occur even without exception case, but without showing the error log "An error occured. Exiting.".
Whether the latter log error is really necessary - I doubt, but I think that it should be clarified within the PR description and title.
##########
PR_DESCRIPTION.md:
##########
Review Comment:
+1 to the above
--
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
Re: [PR] Refactor Bigtable Delete Instance Operator to remove redundant try/excep… [airflow]
Posted by "shahar1 (via GitHub)" <gi...@apache.org>.
shahar1 commented on PR #60727:
URL: https://github.com/apache/airflow/pull/60727#issuecomment-3774905260
> This was my very first PR as I am a rookie in open source and would try to resolve all the issues at the earliest. And I would also like to apologize for my impolite actions and would keep in mind not to repeat them again.
It's alright, apologize accepted!
I'd like to give the original assignee an opportunity to finish working on this file.
Please go over unassigned issues labeled as [good first issue]([good first issue]([good first issue](https://github.com/apache/airflow/issues?q=is%3Aissue%20state%3Aopen%20no%3Aassignee%20label%3A%22good%20first%20issue%22))) and give them a try - I'll be happy to review.
--
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
Re: [PR] Refactor Bigtable Delete Instance Operator to remove redundant try/excep… [airflow]
Posted by "SameerMesiah97 (via GitHub)" <gi...@apache.org>.
SameerMesiah97 commented on PR #60727:
URL: https://github.com/apache/airflow/pull/60727#issuecomment-3764319581
Doesn’t PR #60712 already address this?
--
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