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/12/08 02:26:22 UTC
[GitHub] [airflow] bdsoha opened a new pull request, #28212: Completed D400 for `airflow/kubernetes/*`
bdsoha opened a new pull request, #28212:
URL: https://github.com/apache/airflow/pull/28212
Completed D400 for:
- `airflow/kubernetes/*`
Related to #10742 .
--
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] bdsoha commented on pull request #28212: Completed D400 for `airflow/kubernetes/*`
Posted by GitBox <gi...@apache.org>.
bdsoha commented on PR #28212:
URL: https://github.com/apache/airflow/pull/28212#issuecomment-1342133852
> This was discussed on the devlist a while ago; IIRC the conclusion was to prefer DAG.
Dag is just one example, there are many others.
--
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 #28212: Completed D400 for `airflow/kubernetes/*`
Posted by GitBox <gi...@apache.org>.
uranusjr commented on code in PR #28212:
URL: https://github.com/apache/airflow/pull/28212#discussion_r1042880717
##########
airflow/kubernetes/k8s_model.py:
##########
@@ -25,8 +25,9 @@
class K8SModel(ABC):
"""
- These Airflow Kubernetes models are here for backwards compatibility
- reasons only. Ideally clients should use the kubernetes api
+ Airflow Kubernetes models are here for backwards compatibility reasons only.
+
+ Ideally clients should use the kubernetes api
Review Comment:
```suggestion
Ideally clients should use the kubernetes API
```
##########
airflow/kubernetes/pod_generator.py:
##########
@@ -434,7 +452,7 @@ def deserialize_model_file(path: str) -> k8s.V1Pod:
@staticmethod
def deserialize_model_dict(pod_dict: dict | None) -> k8s.V1Pod:
"""
- Deserializes python dictionary to k8s.V1Pod
+ Deserializes python dictionary to k8s.V1Pod.
Review Comment:
```suggestion
Deserializes a Python dictionary to k8s.V1Pod.
```
##########
airflow/kubernetes/pod_launcher_deprecated.py:
##########
@@ -14,7 +14,7 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
-"""Launches PODs"""
+"""Launches PODs."""
Review Comment:
```suggestion
"""Launches pods."""
```
##########
airflow/kubernetes/pod_launcher_deprecated.py:
##########
@@ -225,7 +228,7 @@ def read_pod_logs(
timestamps: bool = False,
since_seconds: int | None = None,
):
- """Reads log from the POD"""
+ """Reads log from the POD."""
Review Comment:
```suggestion
"""Reads log from the pod."""
```
##########
airflow/kubernetes/pod_launcher_deprecated.py:
##########
@@ -248,7 +251,7 @@ def read_pod_logs(
@tenacity.retry(stop=tenacity.stop_after_attempt(3), wait=tenacity.wait_exponential(), reraise=True)
def read_pod_events(self, pod):
- """Reads events from the POD"""
+ """Reads events from the POD."""
Review Comment:
```suggestion
"""Reads events from the pod."""
```
##########
airflow/kubernetes/pod_launcher_deprecated.py:
##########
@@ -53,7 +53,7 @@
class PodStatus:
- """Status of the PODs"""
+ """Status of the PODs."""
Review Comment:
```suggestion
"""Status of the pods."""
```
##########
airflow/kubernetes/pod_launcher_deprecated.py:
##########
@@ -89,7 +92,7 @@ def __init__(
self.extract_xcom = extract_xcom
def run_pod_async(self, pod: V1Pod, **kwargs):
- """Runs POD asynchronously"""
+ """Runs POD asynchronously."""
Review Comment:
```suggestion
"""Runs pod asynchronously."""
```
##########
airflow/kubernetes/pod_launcher_deprecated.py:
##########
@@ -62,8 +62,10 @@ class PodStatus:
class PodLauncher(LoggingMixin):
- """Deprecated class for launching pods. please use
- airflow.providers.cncf.kubernetes.utils.pod_manager.PodManager instead
+ """
+ Deprecated class for launching pods.
+
+ Please use airflow.providers.cncf.kubernetes.utils.pod_manager.PodManager instead
Review Comment:
```suggestion
Please use airflow.providers.cncf.kubernetes.utils.pod_manager.PodManager instead.
```
##########
airflow/kubernetes/pod_launcher_deprecated.py:
##########
@@ -258,7 +261,7 @@ def read_pod_events(self, pod):
@tenacity.retry(stop=tenacity.stop_after_attempt(3), wait=tenacity.wait_exponential(), reraise=True)
def read_pod(self, pod: V1Pod):
- """Read POD information"""
+ """Read POD information."""
Review Comment:
```suggestion
"""Read pod information."""
```
##########
airflow/kubernetes/pod_launcher_deprecated.py:
##########
@@ -107,7 +110,7 @@ def run_pod_async(self, pod: V1Pod, **kwargs):
return resp
def delete_pod(self, pod: V1Pod):
- """Deletes POD"""
+ """Deletes POD."""
Review Comment:
```suggestion
"""Deletes pod."""
```
##########
airflow/kubernetes/pod_launcher_deprecated.py:
##########
@@ -300,7 +303,7 @@ def _exec_pod_command(self, resp, command):
return None
def process_status(self, job_id, status):
- """Process status information for the JOB"""
+ """Process status information for the JOB."""
Review Comment:
```suggestion
"""Process status information for the job."""
```
--
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 #28212: Completed D400 for `airflow/kubernetes/*`
Posted by GitBox <gi...@apache.org>.
uranusjr commented on PR #28212:
URL: https://github.com/apache/airflow/pull/28212#issuecomment-1342138056
Yeah I think most things do have one most obvious candidate (or at most a couple); we discussed about DAG since it’s specific to Airflow. The main problem is mostly Airflow has a lot of old code that’s historically not as unified (compared to new code) and written in a more YOLO style by people with various literature preferences. It’s probably not that practical to establish rules for each and everything in the code base, but we can fix the small and obvious things when we touch code around them.
--
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] bdsoha commented on pull request #28212: Completed D400 for `airflow/kubernetes/*`
Posted by GitBox <gi...@apache.org>.
bdsoha commented on PR #28212:
URL: https://github.com/apache/airflow/pull/28212#issuecomment-1342113776
> Some nitpicks on spelling.
I completely agree with you. The inconsistency for certain technical terms is something that very much bothered me throughout this process.
For example, which term is correct:
- dag
- Dag
- DAG
- ``dag``
As well as many others....
--
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 #28212: Completed D400 for `airflow/kubernetes/*`
Posted by GitBox <gi...@apache.org>.
uranusjr commented on PR #28212:
URL: https://github.com/apache/airflow/pull/28212#issuecomment-1342120735
This was discussed on the devlist a while ago; IIRC the conclusion was to prefer DAG.
--
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 #28212: Completed D400 for `airflow/kubernetes/*`
Posted by GitBox <gi...@apache.org>.
potiuk merged PR #28212:
URL: https://github.com/apache/airflow/pull/28212
--
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