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 2020/03/22 10:37:07 UTC
[GitHub] [airflow] zhongjiajie opened a new pull request #7802: Make
airflow/providers pylint compatible
zhongjiajie opened a new pull request #7802: Make airflow/providers pylint compatible
URL: https://github.com/apache/airflow/pull/7802
---
Issue link: WILL BE INSERTED BY [boring-cyborg](https://github.com/kaxil/boring-cyborg)
Make sure to mark the boxes below before creating PR: [x]
- [x] Description above provides context of the change
- [x] Unit tests coverage for changes (not needed for documentation changes)
- [x] Commits follow "[How to write a good git commit message](http://chris.beams.io/posts/git-commit/)"
- [x] Relevant documentation is updated including usage instructions.
- [x] I will engage committers as explained in [Contribution Workflow Example](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#contribution-workflow-example).
---
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 [UPDATING.md](https://github.com/apache/airflow/blob/master/UPDATING.md).
Read the [Pull Request Guidelines](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#pull-request-guidelines) for more information.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] zhongjiajie commented on a change in pull request #7802:
Make airflow/providers pylint compatible
Posted by GitBox <gi...@apache.org>.
zhongjiajie commented on a change in pull request #7802: Make airflow/providers pylint compatible
URL: https://github.com/apache/airflow/pull/7802#discussion_r396102522
##########
File path: airflow/providers/opsgenie/operators/opsgenie_alert.py
##########
@@ -92,7 +93,7 @@ def __init__(self,
self.alias = alias
self.description = description
self.responders = responders
- self.visibleTo = visibleTo
+ self.visibleTo = visibleTo # pylint: disable=invalid-name
Review comment:
refactor
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] potiuk commented on issue #7802: Make airflow/providers
pylint compatible
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #7802: Make airflow/providers pylint compatible
URL: https://github.com/apache/airflow/pull/7802#issuecomment-602475098
Thanks @zhongjiajie ! I will take a look at the other requests shortly. I have them on my list. Just wanted to progress with prod image finally :)
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] turbaszek commented on a change in pull request #7802:
Make airflow/providers pylint compatible
Posted by GitBox <gi...@apache.org>.
turbaszek commented on a change in pull request #7802: Make airflow/providers pylint compatible
URL: https://github.com/apache/airflow/pull/7802#discussion_r396082499
##########
File path: airflow/providers/docker/hooks/docker.py
##########
@@ -84,4 +84,4 @@ def __login(self, client):
self.log.debug('Login successful')
except APIError as docker_error:
self.log.error('Docker registry login failed: %s', str(docker_error))
- raise AirflowException('Docker registry login failed: %s', str(docker_error))
+ raise AirflowException('Docker registry login failed: {}'.format(docker_error))
Review comment:
```suggestion
raise AirflowException(f'Docker registry login failed: {docker_error}')
```
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] zhongjiajie commented on a change in pull request #7802:
Make airflow/providers pylint compatible
Posted by GitBox <gi...@apache.org>.
zhongjiajie commented on a change in pull request #7802: Make airflow/providers pylint compatible
URL: https://github.com/apache/airflow/pull/7802#discussion_r396102626
##########
File path: airflow/providers/qubole/hooks/qubole_check.py
##########
@@ -39,14 +42,21 @@ def isint(value):
def isfloat(value):
+ """
+ Whether Qubole column are float
+ """
try:
float(value)
return True
except ValueError:
return False
+# pylint: disable=inconsistent-return-statements
Review comment:
Fix
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] zhongjiajie commented on a change in pull request #7802:
Make airflow/providers pylint compatible
Posted by GitBox <gi...@apache.org>.
zhongjiajie commented on a change in pull request #7802: Make airflow/providers pylint compatible
URL: https://github.com/apache/airflow/pull/7802#discussion_r396102598
##########
File path: airflow/providers/postgres/hooks/postgres.py
##########
@@ -95,7 +96,7 @@ def get_conn(self):
self.conn = psycopg2.connect(**conn_args)
return self.conn
- def copy_expert(self, sql, filename, open=open):
+ def copy_expert(self, sql, filename, open=open): # pylint: disable=redefined-builtin
Review comment:
WDYT
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] turbaszek commented on a change in pull request #7802:
Make airflow/providers pylint compatible
Posted by GitBox <gi...@apache.org>.
turbaszek commented on a change in pull request #7802: Make airflow/providers pylint compatible
URL: https://github.com/apache/airflow/pull/7802#discussion_r396082363
##########
File path: airflow/providers/dingding/hooks/dingding.py
##########
@@ -129,5 +129,5 @@ def send(self):
# Dingding success send message will with errcode equal to 0
if int(resp.json().get('errcode')) != 0:
raise AirflowException('Send Dingding message failed, receive error '
- 'message %s', resp.text)
+ 'message {}'.format(resp.text))
Review comment:
I am in favor of f-strings
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] zhongjiajie commented on a change in pull request #7802:
Make airflow/providers pylint compatible
Posted by GitBox <gi...@apache.org>.
zhongjiajie commented on a change in pull request #7802: Make airflow/providers pylint compatible
URL: https://github.com/apache/airflow/pull/7802#discussion_r396102613
##########
File path: airflow/providers/qubole/hooks/qubole_check.py
##########
@@ -93,17 +109,24 @@ def handle_failure_retry(context):
log.info('Cancelling the Qubole Command Id: %s', cmd_id)
cmd.cancel()
- def get_first(self, sql):
+ def get_first(self):
+ """
+ Get Qubole query first record list
+ """
self.execute(context=self.context)
query_result = self.get_query_results()
row_list = list(filter(None, query_result.split(ROW_DELIM)))
record_list = self.results_parser_callable(row_list)
return record_list
+ # pylint: disable=inconsistent-return-statements
Review comment:
Fix
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] turbaszek commented on a change in pull request #7802:
Make airflow/providers pylint compatible
Posted by GitBox <gi...@apache.org>.
turbaszek commented on a change in pull request #7802: Make airflow/providers pylint compatible
URL: https://github.com/apache/airflow/pull/7802#discussion_r396082221
##########
File path: airflow/providers/apache/pinot/hooks/pinot.py
##########
@@ -66,28 +68,45 @@ def __init__(self,
def get_conn(self):
return self.conn
- def add_schema(self, schema_file, exec=True):
+ def add_schema(self, schema_file: str, with_exec: Optional[bool] = True):
+ """
+ Add Pinot schema by run AddSchema command
+
+ :param schema_file: Pinot schema file
+ :type schema_file: str
+ :param with_exec: bool
+ :type with_exec: Optional[bool]
+ """
cmd = ["AddSchema"]
cmd += ["-controllerHost", self.host]
cmd += ["-controllerPort", self.port]
cmd += ["-schemaFile", schema_file]
- if exec:
+ if with_exec:
cmd += ["-exec"]
self.run_cli(cmd)
- def add_table(self, file_path, exec=True):
+ def add_table(self, file_path: str, with_exec: Optional[bool] = True):
+ """
+ Add Pinot table with run AddTable command
+
+ :param file_path: Pinot table configure file
+ :type file_path: str
+ :param with_exec: bool
+ :type with_exec: Optional[bool]
+ """
cmd = ["AddTable"]
cmd += ["-controllerHost", self.host]
cmd += ["-controllerPort", self.port]
cmd += ["-filePath", file_path]
- if exec:
+ if with_exec:
cmd += ["-exec"]
self.run_cli(cmd)
+ # pylint: disable=too-many-arguments
def create_segment(self,
generator_config_file=None,
data_dir=None,
- format=None,
+ format=None, # pylint: disable=redefined-builtin
Review comment:
Same here, let's refactor it or leave #TODO note
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] zhongjiajie commented on issue #7802: Make
airflow/providers pylint compatible
Posted by GitBox <gi...@apache.org>.
zhongjiajie commented on issue #7802: Make airflow/providers pylint compatible
URL: https://github.com/apache/airflow/pull/7802#issuecomment-602472864
CI green, could you pls task a look @potiuk @turbaszek
BTW, I have other PR for a few days, asking some review in Slack but without respond, could you also give a quick look? https://github.com/apache/airflow/pull/7709 https://github.com/apache/airflow/pull/7683
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] zhongjiajie commented on a change in pull request #7802:
Make airflow/providers pylint compatible
Posted by GitBox <gi...@apache.org>.
zhongjiajie commented on a change in pull request #7802: Make airflow/providers pylint compatible
URL: https://github.com/apache/airflow/pull/7802#discussion_r396102942
##########
File path: airflow/providers/ftp/hooks/ftp.py
##########
@@ -115,7 +115,7 @@ def describe_directory(self, path):
files = dict(mlsd(conn))
return files
- def list_directory(self, path, nlst=False):
Review comment:
So I think we could just remove it directly
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] turbaszek commented on a change in pull request #7802:
Make airflow/providers pylint compatible
Posted by GitBox <gi...@apache.org>.
turbaszek commented on a change in pull request #7802: Make airflow/providers pylint compatible
URL: https://github.com/apache/airflow/pull/7802#discussion_r396082449
##########
File path: airflow/providers/docker/hooks/docker.py
##########
@@ -60,7 +60,7 @@ def __init__(self,
self.__username = conn.login
self.__password = conn.password
self.__email = extra_options.get('email')
- self.__reauth = False if extra_options.get('reauth') == 'no' else True
+ self.__reauth = not extra_options.get('reauth') == 'no'
Review comment:
```suggestion
self.__reauth = extra_options.get('reauth') != 'no'
```
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] zhongjiajie edited a comment on issue #7802: Make
airflow/providers pylint compatible
Posted by GitBox <gi...@apache.org>.
zhongjiajie edited a comment on issue #7802: Make airflow/providers pylint compatible
URL: https://github.com/apache/airflow/pull/7802#issuecomment-602182105
I got it, trying to fix 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] zhongjiajie commented on a change in pull request #7802:
Make airflow/providers pylint compatible
Posted by GitBox <gi...@apache.org>.
zhongjiajie commented on a change in pull request #7802: Make airflow/providers pylint compatible
URL: https://github.com/apache/airflow/pull/7802#discussion_r396102390
##########
File path: airflow/providers/apache/pinot/hooks/pinot.py
##########
@@ -66,28 +68,45 @@ def __init__(self,
def get_conn(self):
return self.conn
- def add_schema(self, schema_file, exec=True):
+ def add_schema(self, schema_file: str, with_exec: Optional[bool] = True):
+ """
+ Add Pinot schema by run AddSchema command
+
+ :param schema_file: Pinot schema file
+ :type schema_file: str
+ :param with_exec: bool
+ :type with_exec: Optional[bool]
+ """
cmd = ["AddSchema"]
cmd += ["-controllerHost", self.host]
cmd += ["-controllerPort", self.port]
cmd += ["-schemaFile", schema_file]
- if exec:
+ if with_exec:
cmd += ["-exec"]
self.run_cli(cmd)
- def add_table(self, file_path, exec=True):
+ def add_table(self, file_path: str, with_exec: Optional[bool] = True):
+ """
+ Add Pinot table with run AddTable command
+
+ :param file_path: Pinot table configure file
+ :type file_path: str
+ :param with_exec: bool
+ :type with_exec: Optional[bool]
+ """
cmd = ["AddTable"]
cmd += ["-controllerHost", self.host]
cmd += ["-controllerPort", self.port]
cmd += ["-filePath", file_path]
- if exec:
+ if with_exec:
cmd += ["-exec"]
self.run_cli(cmd)
+ # pylint: disable=too-many-arguments
def create_segment(self,
generator_config_file=None,
data_dir=None,
- format=None,
+ format=None, # pylint: disable=redefined-builtin
Review comment:
Refactor
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] zhongjiajie commented on issue #7802: Make
airflow/providers pylint compatible
Posted by GitBox <gi...@apache.org>.
zhongjiajie commented on issue #7802: Make airflow/providers pylint compatible
URL: https://github.com/apache/airflow/pull/7802#issuecomment-602389647
Fix conflict
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] zhongjiajie commented on issue #7802: Make
airflow/providers pylint compatible
Posted by GitBox <gi...@apache.org>.
zhongjiajie commented on issue #7802: Make airflow/providers pylint compatible
URL: https://github.com/apache/airflow/pull/7802#issuecomment-602182105
I get it, trying to fix 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] turbaszek commented on a change in pull request #7802:
Make airflow/providers pylint compatible
Posted by GitBox <gi...@apache.org>.
turbaszek commented on a change in pull request #7802: Make airflow/providers pylint compatible
URL: https://github.com/apache/airflow/pull/7802#discussion_r396081526
##########
File path: airflow/providers/apache/druid/hooks/druid.py
##########
@@ -121,7 +127,7 @@ def submit_indexing_job(self, json_index_spec: str):
raise AirflowException('Druid indexing job failed, '
'check console for more info')
else:
- raise AirflowException('Could not get status of the job, got %s', status)
+ raise AirflowException('Could not get status of the job, got {}'.format(status))
Review comment:
```suggestion
raise AirflowException(f'Could not get status of the job, got {status}'))
```
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] potiuk merged pull request #7802: Make airflow/providers
pylint compatible
Posted by GitBox <gi...@apache.org>.
potiuk merged pull request #7802: Make airflow/providers pylint compatible
URL: https://github.com/apache/airflow/pull/7802
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] zhongjiajie commented on a change in pull request #7802:
Make airflow/providers pylint compatible
Posted by GitBox <gi...@apache.org>.
zhongjiajie commented on a change in pull request #7802: Make airflow/providers pylint compatible
URL: https://github.com/apache/airflow/pull/7802#discussion_r396103134
##########
File path: airflow/providers/apache/hive/hooks/hive.py
##########
@@ -633,7 +634,7 @@ def get_databases(self, pattern='*'):
return client.get_databases(pattern)
def get_partitions(
- self, schema, table_name, filter=None):
+ self, schema, table_name, filter=None): # pylint: disable=redefined-builtin
Review comment:
Fix
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] turbaszek commented on a change in pull request #7802:
Make airflow/providers pylint compatible
Posted by GitBox <gi...@apache.org>.
turbaszek commented on a change in pull request #7802: Make airflow/providers pylint compatible
URL: https://github.com/apache/airflow/pull/7802#discussion_r396081835
##########
File path: airflow/providers/apache/hive/hooks/hive.py
##########
@@ -575,10 +579,7 @@ def check_for_partition(self, schema, table, partition):
partitions = client.get_partitions_by_filter(
schema, table, partition, 1)
- if partitions:
- return True
- else:
- return False
+ return partitions
Review comment:
```suggestion
return bool(partitions)
```
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] turbaszek commented on a change in pull request #7802:
Make airflow/providers pylint compatible
Posted by GitBox <gi...@apache.org>.
turbaszek commented on a change in pull request #7802: Make airflow/providers pylint compatible
URL: https://github.com/apache/airflow/pull/7802#discussion_r396081799
##########
File path: airflow/providers/apache/hive/hooks/hive.py
##########
@@ -538,7 +542,7 @@ def sasl_factory():
return hmsclient.HMSClient(iprot=protocol)
- def _find_valid_server(self):
+ def _find_valid_server(self): # pylint: disable=inconsistent-return-statements
Review comment:
I would prefer to fix it instead of disabling it. `return None` at the end should be enough :)
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] turbaszek commented on a change in pull request #7802:
Make airflow/providers pylint compatible
Posted by GitBox <gi...@apache.org>.
turbaszek commented on a change in pull request #7802: Make airflow/providers pylint compatible
URL: https://github.com/apache/airflow/pull/7802#discussion_r396082817
##########
File path: airflow/providers/postgres/hooks/postgres.py
##########
@@ -95,7 +96,7 @@ def get_conn(self):
self.conn = psycopg2.connect(**conn_args)
return self.conn
- def copy_expert(self, sql, filename, open=open):
+ def copy_expert(self, sql, filename, open=open): # pylint: disable=redefined-builtin
Review comment:
Rename or #TODO
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] zhongjiajie commented on issue #7802: Make
airflow/providers pylint compatible
Posted by GitBox <gi...@apache.org>.
zhongjiajie commented on issue #7802: Make airflow/providers pylint compatible
URL: https://github.com/apache/airflow/pull/7802#issuecomment-602474158
BTW, I have to plan to make other pylint compatible in pylint_todo.txt this or next weekend, cause it take some time and I'm not sure if I have 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] potiuk commented on issue #7802: Make airflow/providers
pylint compatible
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #7802: Make airflow/providers pylint compatible
URL: https://github.com/apache/airflow/pull/7802#issuecomment-602181537
Still - there are few static checks 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] turbaszek commented on a change in pull request #7802:
Make airflow/providers pylint compatible
Posted by GitBox <gi...@apache.org>.
turbaszek commented on a change in pull request #7802: Make airflow/providers pylint compatible
URL: https://github.com/apache/airflow/pull/7802#discussion_r396082748
##########
File path: airflow/providers/opsgenie/operators/opsgenie_alert.py
##########
@@ -92,7 +93,7 @@ def __init__(self,
self.alias = alias
self.description = description
self.responders = responders
- self.visibleTo = visibleTo
+ self.visibleTo = visibleTo # pylint: disable=invalid-name
Review comment:
I would be in favor of renaming + note in updating.md
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] zhongjiajie commented on a change in pull request #7802:
Make airflow/providers pylint compatible
Posted by GitBox <gi...@apache.org>.
zhongjiajie commented on a change in pull request #7802: Make airflow/providers pylint compatible
URL: https://github.com/apache/airflow/pull/7802#discussion_r396102567
##########
File path: airflow/providers/postgres/hooks/postgres.py
##########
@@ -95,7 +96,7 @@ def get_conn(self):
self.conn = psycopg2.connect(**conn_args)
return self.conn
- def copy_expert(self, sql, filename, open=open):
+ def copy_expert(self, sql, filename, open=open): # pylint: disable=redefined-builtin
Review comment:
remove it, due to only use in unittest
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] zhongjiajie commented on a change in pull request #7802:
Make airflow/providers pylint compatible
Posted by GitBox <gi...@apache.org>.
zhongjiajie commented on a change in pull request #7802: Make airflow/providers pylint compatible
URL: https://github.com/apache/airflow/pull/7802#discussion_r396102351
##########
File path: airflow/providers/apache/hive/hooks/hive.py
##########
@@ -538,7 +542,7 @@ def sasl_factory():
return hmsclient.HMSClient(iprot=protocol)
- def _find_valid_server(self):
+ def _find_valid_server(self): # pylint: disable=inconsistent-return-statements
Review comment:
Fix
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] zhongjiajie commented on a change in pull request #7802:
Make airflow/providers pylint compatible
Posted by GitBox <gi...@apache.org>.
zhongjiajie commented on a change in pull request #7802: Make airflow/providers pylint compatible
URL: https://github.com/apache/airflow/pull/7802#discussion_r396218172
##########
File path: airflow/providers/ftp/hooks/ftp.py
##########
@@ -115,7 +115,7 @@ def describe_directory(self, path):
files = dict(mlsd(conn))
return files
- def list_directory(self, path, nlst=False):
Review comment:
Fix
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] turbaszek commented on a change in pull request #7802:
Make airflow/providers pylint compatible
Posted by GitBox <gi...@apache.org>.
turbaszek commented on a change in pull request #7802: Make airflow/providers pylint compatible
URL: https://github.com/apache/airflow/pull/7802#discussion_r396082849
##########
File path: airflow/providers/qubole/hooks/qubole_check.py
##########
@@ -93,17 +109,24 @@ def handle_failure_retry(context):
log.info('Cancelling the Qubole Command Id: %s', cmd_id)
cmd.cancel()
- def get_first(self, sql):
+ def get_first(self):
+ """
+ Get Qubole query first record list
+ """
self.execute(context=self.context)
query_result = self.get_query_results()
row_list = list(filter(None, query_result.split(ROW_DELIM)))
record_list = self.results_parser_callable(row_list)
return record_list
+ # pylint: disable=inconsistent-return-statements
Review comment:
Can we fix 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] turbaszek commented on a change in pull request #7802:
Make airflow/providers pylint compatible
Posted by GitBox <gi...@apache.org>.
turbaszek commented on a change in pull request #7802: Make airflow/providers pylint compatible
URL: https://github.com/apache/airflow/pull/7802#discussion_r396082860
##########
File path: airflow/providers/qubole/hooks/qubole_check.py
##########
@@ -39,14 +42,21 @@ def isint(value):
def isfloat(value):
+ """
+ Whether Qubole column are float
+ """
try:
float(value)
return True
except ValueError:
return False
+# pylint: disable=inconsistent-return-statements
Review comment:
Can we fix 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] turbaszek commented on a change in pull request #7802:
Make airflow/providers pylint compatible
Posted by GitBox <gi...@apache.org>.
turbaszek commented on a change in pull request #7802: Make airflow/providers pylint compatible
URL: https://github.com/apache/airflow/pull/7802#discussion_r396104246
##########
File path: airflow/providers/postgres/hooks/postgres.py
##########
@@ -95,7 +96,7 @@ def get_conn(self):
self.conn = psycopg2.connect(**conn_args)
return self.conn
- def copy_expert(self, sql, filename, open=open):
+ def copy_expert(self, sql, filename, open=open): # pylint: disable=redefined-builtin
Review comment:
I am ok with that but then we have to add note in UPDATING.md describing what has changed as this is public interface (someone can use this hook in custom operators).
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] zhongjiajie commented on a change in pull request #7802:
Make airflow/providers pylint compatible
Posted by GitBox <gi...@apache.org>.
zhongjiajie commented on a change in pull request #7802: Make airflow/providers pylint compatible
URL: https://github.com/apache/airflow/pull/7802#discussion_r396102413
##########
File path: airflow/providers/dingding/hooks/dingding.py
##########
@@ -129,5 +129,5 @@ def send(self):
# Dingding success send message will with errcode equal to 0
if int(resp.json().get('errcode')) != 0:
raise AirflowException('Send Dingding message failed, receive error '
- 'message %s', resp.text)
+ 'message {}'.format(resp.text))
Review comment:
Fix
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] turbaszek commented on a change in pull request #7802:
Make airflow/providers pylint compatible
Posted by GitBox <gi...@apache.org>.
turbaszek commented on a change in pull request #7802: Make airflow/providers pylint compatible
URL: https://github.com/apache/airflow/pull/7802#discussion_r396104235
##########
File path: airflow/providers/ftp/hooks/ftp.py
##########
@@ -115,7 +115,7 @@ def describe_directory(self, path):
files = dict(mlsd(conn))
return files
- def list_directory(self, path, nlst=False):
Review comment:
I am ok with that but then we have to add note in UPDATING.md describing what has changed as this is public interface (someone can use this hook in custom operators).
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] turbaszek commented on a change in pull request #7802:
Make airflow/providers pylint compatible
Posted by GitBox <gi...@apache.org>.
turbaszek commented on a change in pull request #7802: Make airflow/providers pylint compatible
URL: https://github.com/apache/airflow/pull/7802#discussion_r396081893
##########
File path: airflow/providers/apache/hive/hooks/hive.py
##########
@@ -633,7 +634,7 @@ def get_databases(self, pattern='*'):
return client.get_databases(pattern)
def get_partitions(
- self, schema, table_name, filter=None):
+ self, schema, table_name, filter=None): # pylint: disable=redefined-builtin
Review comment:
Should we fix it (will need note in UPDATING)? Or at least add TODO note.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] zhongjiajie commented on a change in pull request #7802:
Make airflow/providers pylint compatible
Posted by GitBox <gi...@apache.org>.
zhongjiajie commented on a change in pull request #7802: Make airflow/providers pylint compatible
URL: https://github.com/apache/airflow/pull/7802#discussion_r396102444
##########
File path: airflow/providers/ftp/hooks/ftp.py
##########
@@ -115,7 +115,7 @@ def describe_directory(self, path):
files = dict(mlsd(conn))
return files
- def list_directory(self, path, nlst=False):
Review comment:
This parameter not use in this function
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] turbaszek commented on a change in pull request #7802:
Make airflow/providers pylint compatible
Posted by GitBox <gi...@apache.org>.
turbaszek commented on a change in pull request #7802: Make airflow/providers pylint compatible
URL: https://github.com/apache/airflow/pull/7802#discussion_r396082558
##########
File path: airflow/providers/ftp/hooks/ftp.py
##########
@@ -115,7 +115,7 @@ def describe_directory(self, path):
files = dict(mlsd(conn))
return files
- def list_directory(self, path, nlst=False):
Review comment:
Is this change backward compatible?
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services