[GitHub] [airflow] potiuk commented on a change in pull request #5786: [AIRFLOW-5170] Fix encoding pragmas, consistent licences for python files and related pylint fixes
potiuk commented on a change in pull request #5786: [AIRFLOW-5170] Fix encoding pragmas, consistent licences for python files and related pylint fixes URL: https://github.com/apache/airflow/pull/5786#discussion_r315165506 ## File path: .pre-commit-config.yaml ## @@ -39,6 +48,21 @@ repos: - license-templates/LICENSE.txt files: > \.properties$|\.cfg$|\.conf$|\.ini$|\.ldif$|\.readthedocs$|\.service$|^Dockerfile.*$ + - id: insert-license +name: Add licence for all JS files +files: \.js$ +exclude: ^\.github/.*$|^airflow/_vendor/.*$ +args: + - --comment-style + - "/**| *| */" + - --license-filepath + - license-templates/LICENSE.txt + - repo: https://github.com/pre-commit/pre-commit-hooks +rev: v2.3.0 +hooks: + - id: debug-statements + - id: fix-encoding-pragma +exclude: ^\.github/.*$"|^airflow/_vendor/.*$ Review comment: Right. Makes perfect sense @ashb :) 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [airflow] potiuk commented on a change in pull request #5786: [AIRFLOW-5170] Fix encoding pragmas, consistent licences for python files and related pylint fixes
potiuk commented on a change in pull request #5786: [AIRFLOW-5170] Fix encoding pragmas, consistent licences for python files and related pylint fixes URL: https://github.com/apache/airflow/pull/5786#discussion_r315007104 ## File path: dev/airflow-jira ## @@ -1,21 +1,22 @@ #!/usr/bin/env python Review comment: Not really. The shebang is needed if you want to run it simply as `./airflow-jira` without python in front. It has nothing to do with file ending :). 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [airflow] potiuk commented on a change in pull request #5786: [AIRFLOW-5170] Fix encoding pragmas, consistent licences for python files and related pylint fixes
potiuk commented on a change in pull request #5786: [AIRFLOW-5170] Fix encoding pragmas, consistent licences for python files and related pylint fixes URL: https://github.com/apache/airflow/pull/5786#discussion_r314948556 ## File path: tests/utils/log/elasticmock/__init__.py ## @@ -48,6 +66,7 @@ def _get_elasticmock(hosts=None, *args, **kwargs): def elasticmock(f): Review comment: Right. function is good. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [airflow] potiuk commented on a change in pull request #5786: [AIRFLOW-5170] Fix encoding pragmas, consistent licences for python files and related pylint fixes
potiuk commented on a change in pull request #5786: [AIRFLOW-5170] Fix encoding pragmas, consistent licences for python files and related pylint fixes URL: https://github.com/apache/airflow/pull/5786#discussion_r314948500 ## File path: tests/operators/test_docker_operator.py ## @@ -125,7 +125,7 @@ def test_execute_unicode_logs(self, client_class_mock): client_class_mock.return_value = client_mock -originalRaiseExceptions = logging.raiseExceptions +originalRaiseExceptions = logging.raiseExceptions # pylint: disable=invalid-name Review comment: Built in python functionality: https://docs.python.org/3.5/howto/logging.html#exceptions-raised-during-logging 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [airflow] potiuk commented on a change in pull request #5786: [AIRFLOW-5170] Fix encoding pragmas, consistent licences for python files and related pylint fixes
potiuk commented on a change in pull request #5786: [AIRFLOW-5170] Fix encoding pragmas, consistent licences for python files and related pylint fixes URL: https://github.com/apache/airflow/pull/5786#discussion_r314948451 ## File path: tests/kubernetes/kubernetes_request_factory/test_pod_request_factory.py ## @@ -62,7 +64,7 @@ def setUp(self): 'fsGroup': 2000, } ) -self.maxDiff = None +self.maxDiff = None # pylint: disable=invalid-name Review comment: It's nose's convention to keep maximum number of differences in the output https://kite.com/python/docs/nose.case.Test.maxDiff We can get rid of it if we switch to pytest. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [airflow] potiuk commented on a change in pull request #5786: [AIRFLOW-5170] Fix encoding pragmas, consistent licences for python files and related pylint fixes
potiuk commented on a change in pull request #5786: [AIRFLOW-5170] Fix encoding pragmas, consistent licences for python files and related pylint fixes URL: https://github.com/apache/airflow/pull/5786#discussion_r314948274 ## File path: tests/contrib/utils/gcp_authenticator.py ## @@ -51,20 +51,15 @@ class GcpAuthenticator(LoggingCommandExecutor): """ -Manages authentication to Google Cloud Platform. It helps to manage -connection - it can authenticate with the gcp key name specified -""" +Initialises the authenticator. +:param gcp_key: name of the key to use for authentication (see GCP_*_KEY values) +:param project_extra: optional extra project parameter passed to google cloud + connection Review comment: Done. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [airflow] potiuk commented on a change in pull request #5786: [AIRFLOW-5170] Fix encoding pragmas, consistent licences for python files and related pylint fixes
potiuk commented on a change in pull request #5786: [AIRFLOW-5170] Fix encoding pragmas, consistent licences for python files and related pylint fixes URL: https://github.com/apache/airflow/pull/5786#discussion_r314948234 ## File path: dev/airflow-jira ## @@ -1,21 +1,22 @@ #!/usr/bin/env python Review comment: What do you mean :) ? 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [airflow] potiuk commented on a change in pull request #5786: [AIRFLOW-5170] Fix encoding pragmas, consistent licences for python files and related pylint fixes
potiuk commented on a change in pull request #5786: [AIRFLOW-5170] Fix encoding pragmas, consistent licences for python files and related pylint fixes URL: https://github.com/apache/airflow/pull/5786#discussion_r314948191 ## File path: airflow/utils/log/file_task_handler.py ## @@ -33,13 +33,10 @@ class FileTaskHandler(logging.Handler): task instance logs. It creates and delegates log handling to `logging.FileHandler` after receiving task instance context. It reads logs from task instance's host machine. +:param base_log_folder: Base log folder to place logs. +:param filename_template: template filename string Review comment: Done. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [airflow] potiuk commented on a change in pull request #5786: [AIRFLOW-5170] Fix encoding pragmas, consistent licences for python files and related pylint fixes
potiuk commented on a change in pull request #5786: [AIRFLOW-5170] Fix encoding pragmas, consistent licences for python files and related pylint fixes URL: https://github.com/apache/airflow/pull/5786#discussion_r314948149 ## File path: airflow/security/kerberos.py ## @@ -72,6 +97,11 @@ def renew_from_kt(principal, keytab): def perform_krb181_workaround(principal): +""" +Workaround for Kerberos 1.8.1 Review comment: Added. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [airflow] potiuk commented on a change in pull request #5786: [AIRFLOW-5170] Fix encoding pragmas, consistent licences for python files and related pylint fixes
potiuk commented on a change in pull request #5786: [AIRFLOW-5170] Fix encoding pragmas, consistent licences for python files and related pylint fixes URL: https://github.com/apache/airflow/pull/5786#discussion_r314948148 ## File path: airflow/security/kerberos.py ## @@ -111,6 +141,12 @@ def detect_conf_var(): def run(principal, keytab): +""" +Run the kerbros renewer Review comment: Added. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [airflow] potiuk commented on a change in pull request #5786: [AIRFLOW-5170] Fix encoding pragmas, consistent licences for python files and related pylint fixes
potiuk commented on a change in pull request #5786: [AIRFLOW-5170] Fix encoding pragmas, consistent licences for python files and related pylint fixes URL: https://github.com/apache/airflow/pull/5786#discussion_r314948121 ## File path: airflow/security/kerberos.py ## @@ -28,9 +47,15 @@ def renew_from_kt(principal, keytab): +""" +Renew kerberos token from keytab + +:param principal: principal Review comment: Added. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [airflow] potiuk commented on a change in pull request #5786: [AIRFLOW-5170] Fix encoding pragmas, consistent licences for python files and related pylint fixes
potiuk commented on a change in pull request #5786: [AIRFLOW-5170] Fix encoding pragmas, consistent licences for python files and related pylint fixes URL: https://github.com/apache/airflow/pull/5786#discussion_r314948099 ## File path: airflow/security/kerberos.py ## @@ -1,4 +1,23 @@ #!/usr/bin/env python Review comment: Indeed. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [airflow] potiuk commented on a change in pull request #5786: [AIRFLOW-5170] Fix encoding pragmas, consistent licences for python files and related pylint fixes
potiuk commented on a change in pull request #5786: [AIRFLOW-5170] Fix encoding pragmas, consistent licences for python files and related pylint fixes URL: https://github.com/apache/airflow/pull/5786#discussion_r314948087 ## File path: airflow/operators/docker_operator.py ## @@ -265,6 +276,10 @@ def execute(self, context): self._run_image() def get_command(self): +""" +Retrieve command(s). if command string starts with [, it returns the command list) Review comment: Done. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [airflow] potiuk commented on a change in pull request #5786: [AIRFLOW-5170] Fix encoding pragmas, consistent licences for python files and related pylint fixes
potiuk commented on a change in pull request #5786: [AIRFLOW-5170] Fix encoding pragmas, consistent licences for python files and related pylint fixes URL: https://github.com/apache/airflow/pull/5786#discussion_r314947917 ## File path: airflow/models/baseoperator.py ## @@ -547,12 +553,17 @@ def schedule_interval(self): schedule_interval as it may not be attached to a DAG. """ if self.has_dag(): -return self.dag._schedule_interval +# noinspection PyProtectedMember +return self.dag._schedule_interval # pylint: disable=protected-access else: return self._schedule_interval @property def priority_weight_total(self): +""" +Total priority weight for the operator, all upstream or downstream tasks depending on the weight rule. +:return: calculated priority weight Review comment: I know. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [airflow] potiuk commented on a change in pull request #5786: [AIRFLOW-5170] Fix encoding pragmas, consistent licences for python files and related pylint fixes
potiuk commented on a change in pull request #5786: [AIRFLOW-5170] Fix encoding pragmas, consistent licences for python files and related pylint fixes URL: https://github.com/apache/airflow/pull/5786#discussion_r314947789 ## File path: airflow/kubernetes/pod_launcher.py ## @@ -200,8 +220,10 @@ def _exec_pod_command(self, resp, command): if resp.peek_stderr(): self.log.info(resp.read_stderr()) break +return "" Review comment: Right. Should be None. Good catch. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [airflow] potiuk commented on a change in pull request #5786: [AIRFLOW-5170] Fix encoding pragmas, consistent licences for python files and related pylint fixes
potiuk commented on a change in pull request #5786: [AIRFLOW-5170] Fix encoding pragmas, consistent licences for python files and related pylint fixes URL: https://github.com/apache/airflow/pull/5786#discussion_r314947783 ## File path: airflow/kubernetes/pod_launcher.py ## @@ -14,36 +16,48 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. - +"""Launches PODs""" import json import time -import tenacity from typing import Tuple, Optional +from datetime import datetime as dt +from requests.exceptions import BaseHTTPError + +import tenacity + +from kubernetes import watch, client +from kubernetes.client.rest import ApiException +from kubernetes.stream import stream as kubernetes_stream from airflow.settings import pod_mutation_hook from airflow.utils.log.logging_mixin import LoggingMixin from airflow.utils.state import State -from datetime import datetime as dt from airflow.kubernetes.pod import Pod from airflow.kubernetes.kubernetes_request_factory import pod_request_factory as pod_factory -from kubernetes import watch, client -from kubernetes.client.rest import ApiException -from kubernetes.stream import stream as kubernetes_stream from airflow import AirflowException -from requests.exceptions import BaseHTTPError + from .kube_client import get_kube_client class PodStatus: +"""Status of the PODs""" PENDING = 'pending' RUNNING = 'running' FAILED = 'failed' SUCCEEDED = 'succeeded' class PodLauncher(LoggingMixin): +"""Launches PODS""" def __init__(self, kube_client=None, in_cluster=True, cluster_context=None, extract_xcom=False): +""" +Creates the launcher Review comment: Done. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [airflow] potiuk commented on a change in pull request #5786: [AIRFLOW-5170] Fix encoding pragmas, consistent licences for python files and related pylint fixes
potiuk commented on a change in pull request #5786: [AIRFLOW-5170] Fix encoding pragmas, consistent licences for python files and related pylint fixes URL: https://github.com/apache/airflow/pull/5786#discussion_r314947561 ## File path: airflow/kubernetes/pod.py ## @@ -14,29 +16,35 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. +"""POD definition""" class Resources: +"""POD resources""" def __init__( self, request_memory=None, request_cpu=None, limit_memory=None, limit_cpu=None, limit_gpu=None): +"""Creates port""" Review comment: Yeah. I added also dosctring for the parameters (including types). 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [airflow] potiuk commented on a change in pull request #5786: [AIRFLOW-5170] Fix encoding pragmas, consistent licences for python files and related pylint fixes
potiuk commented on a change in pull request #5786: [AIRFLOW-5170] Fix
encoding pragmas, consistent licences for python files and related pylint fixes
URL: https://github.com/apache/airflow/pull/5786#discussion_r314947157
##
File path: airflow/contrib/hooks/qubole_hook.py
##
@@ -229,28 +236,29 @@ def get_extra_links(self, operator, dttm):
return url
def create_cmd_args(self, context):
+"""Creates command arguments"""
args = []
cmd_type = self.kwargs['command_type']
inplace_args = None
tags = {self.dag_id, self.task_id, context['run_id']}
positional_args_list = flatten_list(POSITIONAL_ARGS.values())
-for k, v in self.kwargs.items():
-if k in COMMAND_ARGS[cmd_type]:
-if k in HYPHEN_ARGS:
-args.append("--{0}={1}".format(k.replace('_', '-'), v))
-elif k in positional_args_list:
-inplace_args = v
-elif k == 'tags':
-if isinstance(v, str):
-tags.add(v)
-elif isinstance(v, (list, tuple)):
-for val in v:
+for key, value in self.kwargs.items(): # pylint:
disable=too-many-nested-blocks
+if key in COMMAND_ARGS[cmd_type]:
+if key in HYPHEN_ARGS:
+args.append("--{0}={1}".format(key.replace('_', '-'),
value))
+elif key in positional_args_list:
+inplace_args = value
+elif key == 'tags':
+if isinstance(value, str):
+tags.add(value)
+elif isinstance(value, (list, tuple)):
+for val in value:
Review comment:
Just tried to minimise the risk of changing anything with such a huge
change. But in this case indeed I can make auto-refactor in IDE making it less
risky.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] potiuk commented on a change in pull request #5786: [AIRFLOW-5170] Fix encoding pragmas, consistent licences for python files and related pylint fixes
potiuk commented on a change in pull request #5786: [AIRFLOW-5170] Fix
encoding pragmas, consistent licences for python files and related pylint fixes
URL: https://github.com/apache/airflow/pull/5786#discussion_r314947099
##
File path: airflow/contrib/hooks/qubole_hook.py
##
@@ -229,28 +236,29 @@ def get_extra_links(self, operator, dttm):
return url
def create_cmd_args(self, context):
+"""Creates command arguments"""
args = []
cmd_type = self.kwargs['command_type']
inplace_args = None
tags = {self.dag_id, self.task_id, context['run_id']}
positional_args_list = flatten_list(POSITIONAL_ARGS.values())
-for k, v in self.kwargs.items():
-if k in COMMAND_ARGS[cmd_type]:
-if k in HYPHEN_ARGS:
-args.append("--{0}={1}".format(k.replace('_', '-'), v))
-elif k in positional_args_list:
-inplace_args = v
-elif k == 'tags':
-if isinstance(v, str):
-tags.add(v)
-elif isinstance(v, (list, tuple)):
-for val in v:
+for key, value in self.kwargs.items(): # pylint:
disable=too-many-nested-blocks
+if key in COMMAND_ARGS[cmd_type]:
+if key in HYPHEN_ARGS:
+args.append("--{0}={1}".format(key.replace('_', '-'),
value))
+elif key in positional_args_list:
+inplace_args = value
+elif key == 'tags':
+if isinstance(value, str):
+tags.add(value)
+elif isinstance(value, (list, tuple)):
+for val in value:
Review comment:
Oh yeah!
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] potiuk commented on a change in pull request #5786: [AIRFLOW-5170] Fix encoding pragmas, consistent licences for python files and related pylint fixes
potiuk commented on a change in pull request #5786: [AIRFLOW-5170] Fix encoding pragmas, consistent licences for python files and related pylint fixes URL: https://github.com/apache/airflow/pull/5786#discussion_r314947018 ## File path: airflow/contrib/hooks/databricks_hook.py ## @@ -41,27 +41,24 @@ class DatabricksHook(BaseHook): """ -Interact with Databricks. Review comment: Mistake! Thanks! 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [airflow] potiuk commented on a change in pull request #5786: [AIRFLOW-5170] Fix encoding pragmas, consistent licences for python files and related pylint fixes
potiuk commented on a change in pull request #5786: [AIRFLOW-5170] Fix encoding pragmas, consistent licences for python files and related pylint fixes URL: https://github.com/apache/airflow/pull/5786#discussion_r314946996 ## File path: CONTRIBUTING.md ## @@ -701,6 +705,10 @@ lint-dockerfile Lint dockerfile mypy Run mypy pylint Run pylint flake8 Run flake8 +insert-license Insert license automatically if missing for python files +debug-statements Check that there are no debug statements committed in python code +fix-encoding-pragma Add missing encoding pragmas in python files +check-hooks-applyCheck that the hooks configured apply to the repository (meta-hook) Review comment: Applied. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [airflow] potiuk commented on a change in pull request #5786: [AIRFLOW-5170] Fix encoding pragmas, consistent licences for python files and related pylint fixes
potiuk commented on a change in pull request #5786: [AIRFLOW-5170] Fix encoding pragmas, consistent licences for python files and related pylint fixes URL: https://github.com/apache/airflow/pull/5786#discussion_r314946968 ## File path: CONTRIBUTING.md ## @@ -701,6 +705,10 @@ lint-dockerfile Lint dockerfile mypy Run mypy pylint Run pylint flake8 Run flake8 +insert-license Insert license automatically if missing for python files +debug-statements Check that there are no debug statements committed in python code +fix-encoding-pragma Add missing encoding pragmas in python files +check-hooks-applyCheck that the hooks configured apply to the repository (meta-hook) Review comment: Why not :) 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [airflow] potiuk commented on a change in pull request #5786: [AIRFLOW-5170] Fix encoding pragmas, consistent licences for python files and related pylint fixes
potiuk commented on a change in pull request #5786: [AIRFLOW-5170] Fix encoding pragmas, consistent licences for python files and related pylint fixes URL: https://github.com/apache/airflow/pull/5786#discussion_r314946961 ## File path: CONTRIBUTING.md ## @@ -618,8 +618,12 @@ as follows: 3) Fix all the issues reported by pylint 4) Re-run [scripts/ci/ci_pylint.sh](scripts/ci/ci_pylint.sh) 5) If you see "success" - submit PR following [Pull Request guidelines](#pull-request-guidelines) +6) You can refresh periodically [scripts/ci/pylint_todo.txt](scripts/ci/pylint_todo.txt). You can do it by + [entering the environment](#entering-bash-shell-in-docker-compose-environment) and running + [scripts/ci/in_container/refresh_pylint_todo.sh](scripts/ci/in_container/refresh_pylint_todo.sh). + This can take quite some time (especially on MacOS)! -There are following guidelines when fixing pylint errors: +You can follow this guidelines when fixing pylint errors: Review comment: applied. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [airflow] potiuk commented on a change in pull request #5786: [AIRFLOW-5170] Fix encoding pragmas, consistent licences for python files and related pylint fixes
potiuk commented on a change in pull request #5786: [AIRFLOW-5170] Fix encoding pragmas, consistent licences for python files and related pylint fixes URL: https://github.com/apache/airflow/pull/5786#discussion_r314946940 ## File path: .pre-commit-config.yaml ## @@ -39,6 +48,21 @@ repos: - license-templates/LICENSE.txt files: > \.properties$|\.cfg$|\.conf$|\.ini$|\.ldif$|\.readthedocs$|\.service$|^Dockerfile.*$ + - id: insert-license +name: Add licence for all JS files +files: \.js$ +exclude: ^\.github/.*$|^airflow/_vendor/.*$ +args: + - --comment-style + - "/**| *| */" + - --license-filepath + - license-templates/LICENSE.txt + - repo: https://github.com/pre-commit/pre-commit-hooks +rev: v2.3.0 +hooks: + - id: debug-statements + - id: fix-encoding-pragma +exclude: ^\.github/.*$"|^airflow/_vendor/.*$ Review comment: Aaaah. I did not realise we do not need them in python 3. Yes. --remove in this case would be nice. But then we touch ALL python files generating potentially lots and lots of conflicts and this will impose fixing all pylint errors for those files. I made sure to fix all the pylint errors in files touched by LICENCE/encoding pragma :). I'd say let's do it as one single big commit after we finish pylint changes ? 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [airflow] potiuk commented on a change in pull request #5786: [AIRFLOW-5170] Fix encoding pragmas, consistent licences for python files and related pylint fixes
potiuk commented on a change in pull request #5786: [AIRFLOW-5170] Fix encoding pragmas, consistent licences for python files and related pylint fixes URL: https://github.com/apache/airflow/pull/5786#discussion_r314556794 ## File path: tests/kubernetes/kubernetes_request_factory/test_pod_request_factory.py ## @@ -62,7 +63,7 @@ def setUp(self): 'fsGroup': 2000, } ) -self.maxDiff = None +self.max_diff = None Review comment: Fixed. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [airflow] potiuk commented on a change in pull request #5786: [AIRFLOW-5170] Fix encoding pragmas, consistent licences for python files and related pylint fixes
potiuk commented on a change in pull request #5786: [AIRFLOW-5170] Fix encoding pragmas, consistent licences for python files and related pylint fixes URL: https://github.com/apache/airflow/pull/5786#discussion_r314556526 ## File path: airflow/contrib/operators/aws_sqs_publish_operator.py ## @@ -16,29 +15,14 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. - +"""Publish message to SQS queue""" from airflow.models import BaseOperator from airflow.utils.decorators import apply_defaults from airflow.contrib.hooks.aws_sqs_hook import SQSHook class SQSPublishOperator(BaseOperator): -""" -Publish message to a SQS queue. - -:param sqs_queue: The SQS queue url (templated) -:type sqs_queue: str -:param message_content: The message content (templated) -:type message_content: str -:param message_attributes: additional attributes for the message (default: None) -For details of the attributes parameter see :py:meth:`botocore.client.SQS.send_message` -:type message_attributes: dict -:param delay_seconds: message delay (templated) (default: 1 second) -:type delay_seconds: int -:param aws_conn_id: AWS connection id (default: aws_default) -:type aws_conn_id: str -""" - +"""Operator to publish message to AWS SQS queue """ Review comment: Right. Fixed few other places as well. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [airflow] potiuk commented on a change in pull request #5786: [AIRFLOW-5170] Fix encoding pragmas, consistent licences for python files and related pylint fixes
potiuk commented on a change in pull request #5786: [AIRFLOW-5170] Fix encoding pragmas, consistent licences for python files and related pylint fixes URL: https://github.com/apache/airflow/pull/5786#discussion_r314551942 ## File path: airflow/contrib/hooks/grpc_hook.py ## @@ -58,7 +61,7 @@ def get_conn(self): if auth_type == "NO_AUTH": channel = grpc.insecure_channel(base_url) -elif auth_type == "SSL" or auth_type == "TLS": +elif auth_type in ["SSL", "TLS"]: Review comment: Sure. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [airflow] potiuk commented on a change in pull request #5786: [AIRFLOW-5170] Fix encoding pragmas, consistent licences for python files and related pylint fixes
potiuk commented on a change in pull request #5786: [AIRFLOW-5170] Fix encoding pragmas, consistent licences for python files and related pylint fixes URL: https://github.com/apache/airflow/pull/5786#discussion_r314551685 ## File path: airflow/contrib/hooks/bigquery_hook.py ## @@ -1,5 +1,4 @@ -# -*- coding: utf-8 -*- # pylint: disable=too-many-lines -# +# -*- coding: utf-8 -*- Review comment: Right. Fixed :). 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
