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/04/09 12:00:11 UTC
[GitHub] [airflow] Bowrna opened a new pull request, #22876: cache and typo fix
Bowrna opened a new pull request, #22876:
URL: https://github.com/apache/airflow/pull/22876
Typo fix in Breeze command line instruction
Cache - use the airflow cache
<!--
Thank you for contributing! Please make sure that your code changes
are covered with tests. And in case of new features or big changes
remember to adjust the documentation.
Feel free to ping committers for the review!
In case of existing issue, reference it using one of the following:
closes: #ISSUE
related: #ISSUE
How to write a good git commit message:
http://chris.beams.io/posts/git-commit/
-->
---
**^ Add meaningful description above**
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst#pull-request-guidelines)** for more information.
In case of fundamental code 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/main/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.
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 #22876: cache and typo fix
Posted by GitBox <gi...@apache.org>.
uranusjr commented on PR #22876:
URL: https://github.com/apache/airflow/pull/22876#issuecomment-1094597360
Needs a rebase though, the doc was rewritten extensively.
--
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] Bowrna commented on pull request #22876: cache and typo fix
Posted by GitBox <gi...@apache.org>.
Bowrna commented on PR #22876:
URL: https://github.com/apache/airflow/pull/22876#issuecomment-1094257729
@potiuk static checks are failing in `sqlalchemy` part. I am not sure why it's failing and the above import of cache from `airflow/compat/functools.py` doesn't have dependency on `sqlalchemy` package. Am I missing to understand any part?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #22876: cache and typo fix
Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #22876:
URL: https://github.com/apache/airflow/pull/22876#issuecomment-1096941420
Ah yeah. I merged it too fast :(
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #22876: cache and typo fix
Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #22876:
URL: https://github.com/apache/airflow/pull/22876#issuecomment-1096942786
Github simply "hidden" the change and I have not realized it's there!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #22876: cache and typo fix
Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #22876:
URL: https://github.com/apache/airflow/pull/22876#issuecomment-1096946514
ah.. I was too fast :)
--
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] github-actions[bot] commented on pull request #22876: cache and typo fix
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #22876:
URL: https://github.com/apache/airflow/pull/22876#issuecomment-1093961429
The PR is likely ready to be merged. No tests are needed as no important environment files, nor python files were modified by it. However, committers might decide that full test matrix is needed and add the 'full tests needed' label. Then you should rebase it to the latest main 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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] Bowrna commented on pull request #22876: cache and typo fix
Posted by GitBox <gi...@apache.org>.
Bowrna commented on PR #22876:
URL: https://github.com/apache/airflow/pull/22876#issuecomment-1094830282
@potiuk @uranusjr
This is the error in static check. This error doesn't occur when i run pre-commit locally. But this occurs in CI. I am not sure how to fix it.
```
Update output of breeze commands in BREEZE.rst...........................................Failed
- hook id: update-breeze-file
- exit code: 1
Traceback (most recent call last):
File "./scripts/ci/pre_commit/pre_commit_breeze_cmd_line.py", line 81, in <module>
print_help_for_all_commands()
File "./scripts/ci/pre_commit/pre_commit_breeze_cmd_line.py", line 46, in print_help_for_all_commands
check_call(["breeze", "--help"], env=env)
File "/opt/hostedtoolcache/Python/3.7.12/x64/lib/python3.7/subprocess.py", line 358, in check_call
retcode = call(*popenargs, **kwargs)
File "/opt/hostedtoolcache/Python/3.7.12/x64/lib/python3.7/subprocess.py", line 339, in call
with Popen(*popenargs, **kwargs) as p:
File "/opt/hostedtoolcache/Python/3.7.12/x64/lib/python3.7/subprocess.py", line 800, in __init__
restore_signals, start_new_session)
File "/opt/hostedtoolcache/Python/3.7.12/x64/lib/python3.7/subprocess.py", line 1551, in _execute_child
raise child_exception_type(errno_num, err_msg, err_filename)
FileNotFoundError: [Errno 2] No such file or directory: 'breeze': 'breeze'
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #22876: cache and typo fix
Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #22876:
URL: https://github.com/apache/airflow/pull/22876#issuecomment-1095072506
I will also add a nicer failure in this check though. It should inform the user that in order to run this check, they need to have breeze installed in the 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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] Bowrna commented on pull request #22876: cache and typo fix
Posted by GitBox <gi...@apache.org>.
Bowrna commented on PR #22876:
URL: https://github.com/apache/airflow/pull/22876#issuecomment-1096323411
:) static basic check is updated with breeze and pipeline is success
--
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] edithturn commented on pull request #22876: cache and typo fix
Posted by GitBox <gi...@apache.org>.
edithturn commented on PR #22876:
URL: https://github.com/apache/airflow/pull/22876#issuecomment-1096942992
I was checking and it's just removing the **Head** and the bottom. I can do it : >>>>>>> 5c42776f29 (cache and typo 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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #22876: cache and typo fix
Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #22876:
URL: https://github.com/apache/airflow/pull/22876#issuecomment-1094259820
@potiuk this code in airflow's functool picks up the cache based on python version airflow/compat/functools.py
Yes. But we always run breeze with Python 3.7, AND breeze should not import "airflow".
This is also the reason for sqlalchemy failing.
Breeze "project" is separate from Airflow - and it should not import "airflow" - because airflow requires far more dependencies. I even just added a pre-commit check for for that in #22880 - (limit-breeze-dependencies) that makes sure that `breeze.py` can imported and run with just `rich` and `click` imported. If you would like Breeze to depend on Airflow - it would mean MANY more dependencies, which we do not want to install for Breeze.
--
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] Bowrna commented on pull request #22876: cache and typo fix
Posted by GitBox <gi...@apache.org>.
Bowrna commented on PR #22876:
URL: https://github.com/apache/airflow/pull/22876#issuecomment-1095000030
@potiuk I have rebased the code now and still issue persists. Am i missing to understand any part?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #22876: cache and typo fix
Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #22876:
URL: https://github.com/apache/airflow/pull/22876#issuecomment-1094018990
We can't use cache from functools as it is Python 3.9+:
https://docs.python.org/3/library/functools.html#functools.cache
> New in version 3.9.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #22876: cache and typo fix
Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #22876:
URL: https://github.com/apache/airflow/pull/22876#issuecomment-1095071539
Ah I see - "Basic checks only"! @uranusjr was right - this CI step should have breeze installation same as the "static checks" - you can copy it from "Static Checks" in "ci.yml"
The problem in this case was an interesting one :)
- you had a change that did not require building image (because only .rst file changed that was not part of the docs)
- therefore instead of "full static chcecks" only "basic ones" triggered
- And the "basic checks" did not have Breeze installed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #22876: cache and typo fix
Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #22876:
URL: https://github.com/apache/airflow/pull/22876#issuecomment-1096945412
Bad me https://github.com/apache/airflow/pull/22953
--
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 #22876: cache and typo fix
Posted by GitBox <gi...@apache.org>.
uranusjr commented on PR #22876:
URL: https://github.com/apache/airflow/pull/22876#issuecomment-1094835825
This seems to be caused by #22880; I think it’s because breeze is not installed in the CI environment.
--
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] edithturn commented on pull request #22876: cache and typo fix
Posted by GitBox <gi...@apache.org>.
edithturn commented on PR #22876:
URL: https://github.com/apache/airflow/pull/22876#issuecomment-1096920830
@Bowrna @potiuk I am trying to understand what is happening, I think there is a conflict in this file https://github.com/apache/airflow/blob/main/BREEZE.rs , it was already merged.
Let me know if this comment doesn't align with what you're already doing.
t
--
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] Bowrna commented on pull request #22876: cache and typo fix
Posted by GitBox <gi...@apache.org>.
Bowrna commented on PR #22876:
URL: https://github.com/apache/airflow/pull/22876#issuecomment-1094256742
> We can't use cache from functools as it is Python 3.9+:
>
> https://docs.python.org/3/library/functools.html#functools.cache
>
> > New in version 3.9.
@potiuk this code in airflow's functool picks up the cache based on python version airflow/compat/functools.py
https://github.com/apache/airflow/blob/0ae0f7e2448e05917e51e29b854ad60463378fbe/airflow/compat/functools.py#L18-L33
--
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] edithturn commented on pull request #22876: cache and typo fix
Posted by GitBox <gi...@apache.org>.
edithturn commented on PR #22876:
URL: https://github.com/apache/airflow/pull/22876#issuecomment-1096947724
yay!! 🕺🏼
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #22876: cache and typo fix
Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #22876:
URL: https://github.com/apache/airflow/pull/22876#issuecomment-1095077690
Added issue for "nicer" message https://github.com/apache/airflow/issues/22908 but for now you can turn this one into PR fixing the problem by adding Breeze installation in the CI job., @edithturn might help if needed.
--
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] edithturn commented on pull request #22876: cache and typo fix
Posted by GitBox <gi...@apache.org>.
edithturn commented on PR #22876:
URL: https://github.com/apache/airflow/pull/22876#issuecomment-1096943872
Let me do it Jarek
--
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] edithturn commented on pull request #22876: cache and typo fix
Posted by GitBox <gi...@apache.org>.
edithturn commented on PR #22876:
URL: https://github.com/apache/airflow/pull/22876#issuecomment-1096943502
I think that should be noted in github-commits before pushing it
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk merged pull request #22876: cache and typo fix
Posted by GitBox <gi...@apache.org>.
potiuk merged PR #22876:
URL: https://github.com/apache/airflow/pull/22876
--
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] edithturn commented on pull request #22876: cache and typo fix
Posted by GitBox <gi...@apache.org>.
edithturn commented on PR #22876:
URL: https://github.com/apache/airflow/pull/22876#issuecomment-1096788841
@Bowrna it should close this one too, right?
I was adding `- run: python -m pip install --editable ./dev/breeze/` in static-checks-basic-checks-only in ci.yaml
https://github.com/apache/airflow/issues/22908
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #22876: cache and typo fix
Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #22876:
URL: https://github.com/apache/airflow/pull/22876#issuecomment-1094908101
Rebase. We have now completely different pre-commit in place of updating the help output.
Instead we generate nice .svg output (using rich + rich-click + rich built-in svg export capabilities) to update the images that are embedded in the BREZEE.rst so the old static check would not work.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on a diff in pull request #22876: cache and typo fix
Posted by GitBox <gi...@apache.org>.
potiuk commented on code in PR #22876:
URL: https://github.com/apache/airflow/pull/22876#discussion_r846770588
##########
dev/breeze/src/airflow_breeze/utils/run_utils.py:
##########
@@ -26,6 +26,7 @@
from pathlib import Path
from typing import Dict, List, Mapping, Optional
+from airflow.compat.functools import cache
Review Comment:
This "import" is wrong. Breeze has no access to "airflow" package and installing "airflow" for Breeze is no-go due to number of dependencies.
--
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] Bowrna commented on a diff in pull request #22876: cache and typo fix
Posted by GitBox <gi...@apache.org>.
Bowrna commented on code in PR #22876:
URL: https://github.com/apache/airflow/pull/22876#discussion_r846774373
##########
dev/breeze/src/airflow_breeze/utils/run_utils.py:
##########
@@ -26,6 +26,7 @@
from pathlib import Path
from typing import Dict, List, Mapping, Optional
+from airflow.compat.functools import cache
Review Comment:
i see. i could understand this now. thanks @potiuk i will revert the 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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] Bowrna commented on pull request #22876: cache and typo fix
Posted by GitBox <gi...@apache.org>.
Bowrna commented on PR #22876:
URL: https://github.com/apache/airflow/pull/22876#issuecomment-1094277406
@potiuk @uranusjr I have reverted the cache related changes. That leaves this PR with small typo fix alone. But i got to learn why i shouldn't import from airflow package. :)
--
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] Bowrna commented on pull request #22876: cache and typo fix
Posted by GitBox <gi...@apache.org>.
Bowrna commented on PR #22876:
URL: https://github.com/apache/airflow/pull/22876#issuecomment-1094995271
Could you explain more about .svg @potiuk ? You could share any relevant docs or point to links to know more about this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org