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 2021/02/21 13:07:45 UTC
[GitHub] [airflow] ephraimbuddy opened a new pull request #14344: Fix Incorrect warning in upgrade check and error in reading file
ephraimbuddy opened a new pull request #14344:
URL: https://github.com/apache/airflow/pull/14344
Closes: #14340
Apart from the above issue fixed, I also fixed an issue that didn't allow me run upgrade_check in breeze. When I ran it, I got error:
```
Traceback (most recent call last):
File "/usr/local/bin/airflow", line 7, in <module>
exec(compile(f.read(), __file__, 'exec'))
File "/opt/airflow/airflow/bin/airflow", line 37, in <module>
args.func(args)
File "/opt/airflow/airflow/upgrade/checker.py", line 118, in run
all_problems = check_upgrade(formatter, rules)
File "/opt/airflow/airflow/upgrade/checker.py", line 38, in check_upgrade
rule_status = RuleStatus.from_rule(rule)
File "/opt/airflow/airflow/upgrade/problem.py", line 44, in from_rule
result = rule.check()
File "/opt/airflow/airflow/upgrade/rules/chain_between_dag_and_operator_not_allowed_rule.py", line 72, in check
problems.extend(self._check_file(file_path))
File "/opt/airflow/airflow/upgrade/rules/chain_between_dag_and_operator_not_allowed_rule.py", line 40, in _check_file
lines = file_pointer.readlines()
File "/usr/local/lib/python3.7/codecs.py", line 322, in decode
(result, consumed) = self._buffer_decode(data, self.errors, final)
UnicodeDecodeError: 'utf-8' codec can't decode byte 0x94 in position 22: invalid start byte
```
I figured we can ignore the read error since it works fine outside breeze.
---
**^ Add meaningful description above**
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#pull-request-guidelines)** for more information.
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).
----------------------------------------------------------------
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
[GitHub] [airflow] turbaszek commented on a change in pull request #14344: Fix Incorrect warning in upgrade check and error in reading file
Posted by GitBox <gi...@apache.org>.
turbaszek commented on a change in pull request #14344:
URL: https://github.com/apache/airflow/pull/14344#discussion_r580226458
##########
File path: airflow/upgrade/rules/import_changes.py
##########
@@ -17,7 +17,7 @@
import itertools
from typing import NamedTuple, Optional, List
-
+import os.path
Review comment:
```suggestion
import os
```
----------------------------------------------------------------
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
[GitHub] [airflow] ephraimbuddy commented on a change in pull request #14344: Fix Incorrect warning in upgrade check and error in reading file
Posted by GitBox <gi...@apache.org>.
ephraimbuddy commented on a change in pull request #14344:
URL: https://github.com/apache/airflow/pull/14344#discussion_r580068159
##########
File path: airflow/upgrade/rules/chain_between_dag_and_operator_not_allowed_rule.py
##########
@@ -36,7 +36,7 @@ def _change_info(self, file_path, line_number):
def _check_file(self, file_path):
problems = []
- with open(file_path, "r") as file_pointer:
+ with open(file_path, "r", errors="ignore") as file_pointer:
Review comment:
Yeah. Fixed. Please take a look https://github.com/apache/airflow/pull/14344/commits/03b2581fcb4f27b2000e6c4490246d07148304e9
----------------------------------------------------------------
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
[GitHub] [airflow] github-actions[bot] commented on pull request #14344: Fix Incorrect warning in upgrade check and error in reading file
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #14344:
URL: https://github.com/apache/airflow/pull/14344#issuecomment-783129875
[The Workflow run](https://github.com/apache/airflow/actions/runs/588046422) is cancelling this PR. Building images for the PR has failed. Follow the the workflow link to check the reason.
----------------------------------------------------------------
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
[GitHub] [airflow] ephraimbuddy commented on a change in pull request #14344: Fix Incorrect warning in upgrade check and error in reading file
Posted by GitBox <gi...@apache.org>.
ephraimbuddy commented on a change in pull request #14344:
URL: https://github.com/apache/airflow/pull/14344#discussion_r579805195
##########
File path: airflow/upgrade/rules/db_api_functions.py
##########
@@ -33,20 +33,16 @@ def check_run(cls):
try:
cls.__new__(cls).run("fake SQL")
return return_error_string(cls, "run")
- except NotImplementedError:
- pass
except Exception:
- return return_error_string(cls, "run")
+ pass
Review comment:
This was the culprit as attribute error due to not finding a connection gets returned as run implementation
----------------------------------------------------------------
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
[GitHub] [airflow] ephraimbuddy commented on pull request #14344: Fix Incorrect warning in upgrade check and error in reading file
Posted by GitBox <gi...@apache.org>.
ephraimbuddy commented on pull request #14344:
URL: https://github.com/apache/airflow/pull/14344#issuecomment-783174009
Added a fix for #14258 since it's just one line change https://github.com/apache/airflow/pull/13012#discussion_r576869862 @eladkal
----------------------------------------------------------------
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
[GitHub] [airflow] eladkal commented on a change in pull request #14344: Fix Incorrect warning in upgrade check and error in reading file
Posted by GitBox <gi...@apache.org>.
eladkal commented on a change in pull request #14344:
URL: https://github.com/apache/airflow/pull/14344#discussion_r580118168
##########
File path: airflow/upgrade/rules/chain_between_dag_and_operator_not_allowed_rule.py
##########
@@ -36,7 +36,7 @@ def _change_info(self, file_path, line_number):
def _check_file(self, file_path):
problems = []
- with open(file_path, "r") as file_pointer:
+ with open(file_path, "r", errors="ignore") as file_pointer:
Review comment:
Do you need also a "Unable to read python file" message like added in https://github.com/apache/airflow/pull/13371 ?
----------------------------------------------------------------
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
[GitHub] [airflow] ephraimbuddy commented on pull request #14344: Fix Incorrect warning in upgrade check and error in reading file
Posted by GitBox <gi...@apache.org>.
ephraimbuddy commented on pull request #14344:
URL: https://github.com/apache/airflow/pull/14344#issuecomment-782931537
> so you also fixed #14243 on the way :)
Wow...interesting.
Cool :+1:
----------------------------------------------------------------
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
[GitHub] [airflow] kaxil merged pull request #14344: Fix Incorrect warning in upgrade check and error in reading file
Posted by GitBox <gi...@apache.org>.
kaxil merged pull request #14344:
URL: https://github.com/apache/airflow/pull/14344
----------------------------------------------------------------
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
[GitHub] [airflow] turbaszek commented on a change in pull request #14344: Fix Incorrect warning in upgrade check and error in reading file
Posted by GitBox <gi...@apache.org>.
turbaszek commented on a change in pull request #14344:
URL: https://github.com/apache/airflow/pull/14344#discussion_r580046818
##########
File path: airflow/upgrade/rules/chain_between_dag_and_operator_not_allowed_rule.py
##########
@@ -36,7 +36,7 @@ def _change_info(self, file_path, line_number):
def _check_file(self, file_path):
problems = []
- with open(file_path, "r") as file_pointer:
+ with open(file_path, "r", errors="ignore") as file_pointer:
Review comment:
I think this may solve the problem. To be honest I'm not sure if ignoring errors is a good idea - it may make hard spotting a next 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] turbaszek commented on a change in pull request #14344: Fix Incorrect warning in upgrade check and error in reading file
Posted by GitBox <gi...@apache.org>.
turbaszek commented on a change in pull request #14344:
URL: https://github.com/apache/airflow/pull/14344#discussion_r580044907
##########
File path: airflow/upgrade/rules/chain_between_dag_and_operator_not_allowed_rule.py
##########
@@ -36,7 +36,7 @@ def _change_info(self, file_path, line_number):
def _check_file(self, file_path):
problems = []
- with open(file_path, "r") as file_pointer:
+ with open(file_path, "r", errors="ignore") as file_pointer:
Review comment:
Should we check file extension before opening it and open only `.py` files?
----------------------------------------------------------------
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
[GitHub] [airflow] turbaszek commented on a change in pull request #14344: Fix Incorrect warning in upgrade check and error in reading file
Posted by GitBox <gi...@apache.org>.
turbaszek commented on a change in pull request #14344:
URL: https://github.com/apache/airflow/pull/14344#discussion_r580226658
##########
File path: airflow/upgrade/rules/chain_between_dag_and_operator_not_allowed_rule.py
##########
@@ -17,7 +17,7 @@
from __future__ import absolute_import
import re
-
+import os.path
Review comment:
```suggestion
import os
```
We usually use `import os`
----------------------------------------------------------------
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
[GitHub] [airflow] eladkal commented on pull request #14344: Fix Incorrect warning in upgrade check and error in reading file
Posted by GitBox <gi...@apache.org>.
eladkal commented on pull request #14344:
URL: https://github.com/apache/airflow/pull/14344#issuecomment-782902821
so you also fixed https://github.com/apache/airflow/issues/14243 on the way :)
----------------------------------------------------------------
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
[GitHub] [airflow] ephraimbuddy commented on a change in pull request #14344: Fix Incorrect warning in upgrade check and error in reading file
Posted by GitBox <gi...@apache.org>.
ephraimbuddy commented on a change in pull request #14344:
URL: https://github.com/apache/airflow/pull/14344#discussion_r580280036
##########
File path: airflow/upgrade/rules/chain_between_dag_and_operator_not_allowed_rule.py
##########
@@ -36,7 +36,7 @@ def _change_info(self, file_path, line_number):
def _check_file(self, file_path):
problems = []
- with open(file_path, "r") as file_pointer:
+ with open(file_path, "r", errors="ignore") as file_pointer:
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:
users@infra.apache.org
[GitHub] [airflow] ephraimbuddy commented on pull request #14344: Fix Incorrect warning in upgrade check and error in reading file
Posted by GitBox <gi...@apache.org>.
ephraimbuddy commented on pull request #14344:
URL: https://github.com/apache/airflow/pull/14344#issuecomment-784900587
Hi @turbaszek, can you take a look once more
----------------------------------------------------------------
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
[GitHub] [airflow] github-actions[bot] commented on pull request #14344: Fix Incorrect warning in upgrade check and error in reading file
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #14344:
URL: https://github.com/apache/airflow/pull/14344#issuecomment-785080777
The PR most likely needs to run full matrix of tests because it modifies parts of the core of Airflow. However, committers might decide to merge it quickly and take the risk. If they don't merge it quickly - please rebase it to the latest master at your convenience, or amend the last commit of the PR, and push it with --force-with-lease.
----------------------------------------------------------------
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
[GitHub] [airflow] ephraimbuddy commented on a change in pull request #14344: Fix Incorrect warning in upgrade check and error in reading file
Posted by GitBox <gi...@apache.org>.
ephraimbuddy commented on a change in pull request #14344:
URL: https://github.com/apache/airflow/pull/14344#discussion_r580068159
##########
File path: airflow/upgrade/rules/chain_between_dag_and_operator_not_allowed_rule.py
##########
@@ -36,7 +36,7 @@ def _change_info(self, file_path, line_number):
def _check_file(self, file_path):
problems = []
- with open(file_path, "r") as file_pointer:
+ with open(file_path, "r", errors="ignore") as file_pointer:
Review comment:
Yeah. Fixed. Please take a look https://github.com/apache/airflow/pull/14344/commits/9e1298a8758d512cb828c112ac98409d98e9ef4d
----------------------------------------------------------------
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