You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "ferruzzi (via GitHub)" <gi...@apache.org> on 2023/07/05 03:40:34 UTC
[GitHub] [airflow] ferruzzi opened a new pull request, #32356: D205 Support - Provider: Google
ferruzzi opened a new pull request, #32356:
URL: https://github.com/apache/airflow/pull/32356
Part of https://github.com/apache/airflow/issues/10742
D205 asserts that all docstrings must have a one-line summary ending in a period. If there is more than one sentence then there must be a blank line before the rest of the docstring. Meeting these requirements could be as simple as adding a newline, or might require some rephrasing.
There are almost a thousand violations in the repo so we're going to have to take this in bites.
### PLEASE NOTE
There should be zero logic changes in this PR, only changes to docstrings and whitespace. If you see otherwise, please call it out.
### Included in this chunk
All files contained in the Google provider package
### To test
If you comment out [this line](https://github.com/apache/airflow/blob/main/pyproject.toml#L68) and run pre-commit in main you will get around 642 errors. After these changes, "only" 359 remain and no files in any of the above provider packages should be on the list. After uncommenting that line and rerunning pre-commits, there should be zero regressions.
<!--
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 an 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/
-->
---
**^ 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 changes, an Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvement+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 newsfragment file, named `{pr_number}.significant.rst` or `{issue_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] uranusjr commented on pull request #32356: D205 Support - Provider: Google
Posted by "uranusjr (via GitHub)" <gi...@apache.org>.
uranusjr commented on PR #32356:
URL: https://github.com/apache/airflow/pull/32356#issuecomment-1623078940
Docstrings in the Google provider is a tower of cards that collapse if you touch it. I don’t have anything good to say about it except good luck… splitting the PR into 10 could be a good idea to isolate the issue (although it may not help you understand _why_ there’s an issue).
--
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] ferruzzi commented on pull request #32356: D205 Support - Provider: Google
Posted by "ferruzzi (via GitHub)" <gi...@apache.org>.
ferruzzi commented on PR #32356:
URL: https://github.com/apache/airflow/pull/32356#issuecomment-1622195284
Thanks for the tip. Sometimes it's one of those things where someone else will look and say "on, there's an extra space there at the beginning of line 20" or something. I was hoping it was one of those days :P I'll start poking at it this evening and see what I cans see.
--
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] ferruzzi commented on a diff in pull request #32356: D205 Support - Provider: Google
Posted by "ferruzzi (via GitHub)" <gi...@apache.org>.
ferruzzi commented on code in PR #32356:
URL: https://github.com/apache/airflow/pull/32356#discussion_r1257338970
##########
airflow/providers/google/cloud/operators/dataflow.py:
##########
@@ -1018,12 +1018,12 @@ class DataflowCreatePythonJobOperator(GoogleCloudBaseOperator):
"""
Launching Cloud Dataflow jobs written in python.
- Note that both dataflow_default_options and options will be merged to specify pipeline
Review Comment:
There was a leading space in that line which was causing the indent error. 🤦 That was like playing "find the misplaced semicolon" LOL
--
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] ferruzzi commented on pull request #32356: D205 Support - Provider: Google
Posted by "ferruzzi (via GitHub)" <gi...@apache.org>.
ferruzzi commented on PR #32356:
URL: https://github.com/apache/airflow/pull/32356#issuecomment-1623940364
> splitting the PR into 10
I may do just that. I was trying to keep it in reasonable chunks rather than flooding a hundred PRs but I think you are right on this one. I'll try to get to it tonight, I didn't have time last night to look into it.
--
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] ferruzzi commented on pull request #32356: D205 Support - Provider: Google
Posted by "ferruzzi (via GitHub)" <gi...@apache.org>.
ferruzzi commented on PR #32356:
URL: https://github.com/apache/airflow/pull/32356#issuecomment-1627469303
Alright, everything is passing locally now, let's see if I did the right magic incantation this time.
--
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] ferruzzi commented on pull request #32356: D205 Support - Provider: Google
Posted by "ferruzzi (via GitHub)" <gi...@apache.org>.
ferruzzi commented on PR #32356:
URL: https://github.com/apache/airflow/pull/32356#issuecomment-1622127320
> Some build docs failure.
Yeah, I saw those locally but I'm not sure what they mean yet. I'll need to poke at them tonight I guess unless it's obvious to someone else.
--
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] ferruzzi commented on a diff in pull request #32356: D205 Support - Provider: Google
Posted by "ferruzzi (via GitHub)" <gi...@apache.org>.
ferruzzi commented on code in PR #32356:
URL: https://github.com/apache/airflow/pull/32356#discussion_r1257338970
##########
airflow/providers/google/cloud/operators/dataflow.py:
##########
@@ -1018,12 +1018,12 @@ class DataflowCreatePythonJobOperator(GoogleCloudBaseOperator):
"""
Launching Cloud Dataflow jobs written in python.
- Note that both dataflow_default_options and options will be merged to specify pipeline
Review Comment:
There was a [redacted] [redacted] [redacted] leading space in that line which was causing the indent error. 🤦 That was like playing "find the misplaced semicolon" LOL
##########
airflow/providers/google/cloud/operators/dataflow.py:
##########
@@ -1018,12 +1018,12 @@ class DataflowCreatePythonJobOperator(GoogleCloudBaseOperator):
"""
Launching Cloud Dataflow jobs written in python.
- Note that both dataflow_default_options and options will be merged to specify pipeline
Review Comment:
There was a [redacted] [redacted] [redacted] leading space in that line which was causing the indent error. 🤦 That was like playing "find the misplaced semicolon" LOL
--
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] ferruzzi closed pull request #32356: D205 Support - Provider: Google
Posted by "ferruzzi (via GitHub)" <gi...@apache.org>.
ferruzzi closed pull request #32356: D205 Support - Provider: Google
URL: https://github.com/apache/airflow/pull/32356
--
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] ferruzzi commented on a diff in pull request #32356: D205 Support - Provider: Google
Posted by "ferruzzi (via GitHub)" <gi...@apache.org>.
ferruzzi commented on code in PR #32356:
URL: https://github.com/apache/airflow/pull/32356#discussion_r1256730741
##########
airflow/providers/google/cloud/operators/dataflow.py:
##########
@@ -1015,11 +1016,11 @@ def on_kill(self) -> None:
class DataflowCreatePythonJobOperator(GoogleCloudBaseOperator):
"""
- Launching Cloud Dataflow jobs written in python. Note that both
- dataflow_default_options and options will be merged to specify pipeline
- execution parameter, and dataflow_default_options is expected to save
- high-level options, for instances, project and zone information, which
- apply to all dataflow operators in the DAG.
+ Launching Cloud Dataflow jobs written in python.
+
+ Note that both dataflow_default_options and options will be merged to specify pipeline
+ execution parameter, and dataflow_default_options is expected to save high-level options,
+ for instances, project and zone information, which apply to all dataflow operators in the DAG.
This class is deprecated.
Please use `providers.apache.beam.operators.beam.BeamRunPythonPipelineOperator`.
Review Comment:
@uranusjr @potiuk A number of these classed are deprecated. Do we still want/need these complicated docstrings for them? I'm guessing we do since the classes are still there and functional, huh? That "This c;ass is deprecated" note is from [this commit](https://github.com/apache/airflow/pull/12814) back in 2021
--
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 #32356: D205 Support - Provider: Google
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #32356:
URL: https://github.com/apache/airflow/pull/32356#issuecomment-1621072806
Some build docs failure.
--
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 #32356: D205 Support - Provider: Google
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #32356:
URL: https://github.com/apache/airflow/pull/32356#discussion_r1257341941
##########
airflow/providers/google/cloud/operators/dataflow.py:
##########
@@ -1018,12 +1018,12 @@ class DataflowCreatePythonJobOperator(GoogleCloudBaseOperator):
"""
Launching Cloud Dataflow jobs written in python.
- Note that both dataflow_default_options and options will be merged to specify pipeline
Review Comment:
BEEN THERE DONE 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
[GitHub] [airflow] potiuk merged pull request #32356: D205 Support - Provider: Google
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk merged PR #32356:
URL: https://github.com/apache/airflow/pull/32356
--
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] ferruzzi commented on a diff in pull request #32356: D205 Support - Provider: Google
Posted by "ferruzzi (via GitHub)" <gi...@apache.org>.
ferruzzi commented on code in PR #32356:
URL: https://github.com/apache/airflow/pull/32356#discussion_r1256730741
##########
airflow/providers/google/cloud/operators/dataflow.py:
##########
@@ -1015,11 +1016,11 @@ def on_kill(self) -> None:
class DataflowCreatePythonJobOperator(GoogleCloudBaseOperator):
"""
- Launching Cloud Dataflow jobs written in python. Note that both
- dataflow_default_options and options will be merged to specify pipeline
- execution parameter, and dataflow_default_options is expected to save
- high-level options, for instances, project and zone information, which
- apply to all dataflow operators in the DAG.
+ Launching Cloud Dataflow jobs written in python.
+
+ Note that both dataflow_default_options and options will be merged to specify pipeline
+ execution parameter, and dataflow_default_options is expected to save high-level options,
+ for instances, project and zone information, which apply to all dataflow operators in the DAG.
This class is deprecated.
Please use `providers.apache.beam.operators.beam.BeamRunPythonPipelineOperator`.
Review Comment:
@uranusjr @potiuk A number of these classed are deprecated. Do we still want/need these complicated docstrings for them? I'm guessing we do since the classes are still there and functional, huh? That "This c;ass is deprecated" note is from [this commit](https://github.com/apache/airflow/pull/12814) back in 2021
--
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 #32356: D205 Support - Provider: Google
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #32356:
URL: https://github.com/apache/airflow/pull/32356#issuecomment-1622144492
Obviously those are blank indentation issues in docs generated via autoapi that are not-too-obvious where they are coming from as the autoapi+sphinx are known to speak in riddles.
You might have to take a look at generated .rst files in `_api` folder and hope that you will find out quickly. Other than that, I sometimes had to revert to bisecting to narrow down similar problems and remove parts of my modifications to be able to pin-point the places they were coming from,
Usually it SOME problem with identation that docstrings processesed by Autoapi make Sphinx very unhappy about.
--
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 #32356: D205 Support - Provider: Google
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #32356:
URL: https://github.com/apache/airflow/pull/32356#issuecomment-1622146755
(and sorry I can't be more helpful :).
--
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 #32356: D205 Support - Provider: Google
Posted by "uranusjr (via GitHub)" <gi...@apache.org>.
uranusjr commented on code in PR #32356:
URL: https://github.com/apache/airflow/pull/32356#discussion_r1253998420
##########
airflow/providers/google/common/hooks/base_google.py:
##########
@@ -497,8 +488,11 @@ def wrapper(self: GoogleBaseHook, *args, **kwargs):
@contextmanager
def provide_gcp_credential_file_as_context(self) -> Generator[str | None, None, None]:
"""
- Context manager that provides a Google Cloud credentials for application supporting `Application
- Default Credentials (ADC) strategy <https://cloud.google.com/docs/authentication/production>`__.
+ Provides a Google Cloud credentials for Application Default Credentials (ADC) strategy support.
+
+ See:
+ `Application Default Credentials (ADC)
+ strategy <https://cloud.google.com/docs/authentication/production>`__.
Review Comment:
Use `.. seealso::` for 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