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/02/05 19:37:07 UTC
[GitHub] [airflow] Melodie97 opened a new pull request #21346: Update docs/readme.rst
Melodie97 opened a new pull request #21346:
URL: https://github.com/apache/airflow/pull/21346
<!--
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] Melodie97 commented on a change in pull request #21346: Update docs/readme.rst
Posted by GitBox <gi...@apache.org>.
Melodie97 commented on a change in pull request #21346:
URL: https://github.com/apache/airflow/pull/21346#discussion_r803059338
##########
File path: docs/start_doc_server.sh
##########
@@ -20,5 +20,11 @@ DOCS_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"
readonly DOCS_DIR
(cd "${DOCS_DIR}"/_build || exit;
- python -m http.server 8000
+ version=$(python -V 2>&1 | grep -Po '(?<=Python )(.+)')
+ if [[ -z "$version" ]]
+ then
+ python3 -m http.server 8000
Review comment:
ok
--
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 change in pull request #21346: Update docs/readme.rst
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #21346:
URL: https://github.com/apache/airflow/pull/21346#discussion_r802614941
##########
File path: docs/start_doc_server.sh
##########
@@ -20,5 +20,11 @@ DOCS_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"
readonly DOCS_DIR
(cd "${DOCS_DIR}"/_build || exit;
- python -m http.server 8000
+ version=$(python -V 2>&1 | grep -Po '(?<=Python )(.+)')
+ if [[ -z "$version" ]]
+ then
+ python3 -m http.server 8000
Review comment:
Well actually just replacing `python` with `python3 in the original command is enoug. No need to ad "version", "if" etc.
```
python -m http.server 8000 => python3 -m http.server 8000
```
--
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] Melodie97 commented on a change in pull request #21346: Update docs/readme.rst
Posted by GitBox <gi...@apache.org>.
Melodie97 commented on a change in pull request #21346:
URL: https://github.com/apache/airflow/pull/21346#discussion_r801582538
##########
File path: docs/start_doc_server.sh
##########
@@ -20,5 +20,11 @@ DOCS_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"
readonly DOCS_DIR
(cd "${DOCS_DIR}"/_build || exit;
- python -m http.server 8000
+ version=$(python -V 2>&1 | grep -Po '(?<=Python )(.+)')
+ if [[ -z "$version" ]]
+ then
+ python3 -m http.server 8000
Review comment:
ok I should just replace the python with python3?
--
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 #21346: Update docs/readme.rst
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #21346:
URL: https://github.com/apache/airflow/pull/21346#issuecomment-1030687370
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] Melodie97 commented on a change in pull request #21346: Update docs/readme.rst
Posted by GitBox <gi...@apache.org>.
Melodie97 commented on a change in pull request #21346:
URL: https://github.com/apache/airflow/pull/21346#discussion_r800454840
##########
File path: docs/README.rst
##########
@@ -74,7 +74,24 @@ Once you have built the documentation run the following command from the root di
docs/start_doc_server.sh
-Then, view your docs at ``localhost:8000``
+You need to have python installed to run the command, if you get an error message -- python: command not found
+It is possible that you don't have python installed or added to PATH.
+Visit link <https://python.land/installing-python> to install python and add to PATH
Review comment:
ok
--
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] eladkal commented on a change in pull request #21346: Update docs/readme.rst
Posted by GitBox <gi...@apache.org>.
eladkal commented on a change in pull request #21346:
URL: https://github.com/apache/airflow/pull/21346#discussion_r800148607
##########
File path: docs/README.rst
##########
@@ -74,7 +74,24 @@ Once you have built the documentation run the following command from the root di
docs/start_doc_server.sh
-Then, view your docs at ``localhost:8000``
+You need to have python installed to run the command, if you get an error message -- python: command not found
+It is possible that you don't have python installed or added to PATH.
+Visit link <https://python.land/installing-python> to install python and add to PATH
Review comment:
```suggestion
You need to have python installed to run the command, if you get an error message ``-- python: command not found``
It is possible that you don't have python installed or added to PATH.
Visit `link <https://python.land/installing-python>`_ to install python and add to PATH
```
--
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 change in pull request #21346: Update docs/readme.rst
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #21346:
URL: https://github.com/apache/airflow/pull/21346#discussion_r802615249
##########
File path: docs/start_doc_server.sh
##########
@@ -20,5 +20,11 @@ DOCS_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"
readonly DOCS_DIR
(cd "${DOCS_DIR}"/_build || exit;
- python -m http.server 8000
+ version=$(python -V 2>&1 | grep -Po '(?<=Python )(.+)')
+ if [[ -z "$version" ]]
+ then
+ python3 -m http.server 8000
Review comment:
That's it - much simpler and it will work in all cases.
--
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 change in pull request #21346: Update docs/readme.rst
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #21346:
URL: https://github.com/apache/airflow/pull/21346#discussion_r800551393
##########
File path: docs/README.rst
##########
@@ -74,7 +74,24 @@ Once you have built the documentation run the following command from the root di
docs/start_doc_server.sh
-Then, view your docs at ``localhost:8000``
+You need to have python installed to run the command, if you get an error message -- python: command not found
+It is possible that you don't have python installed or added to PATH.
+Visit link <https://python.land/installing-python> to install python and add to PATH
Review comment:
One more thing. I think it will be better if you remove the whole chapter about "replacing python with python3" - but instead you should simply **actually replace** it as part of this PR.
I believe while "python" might not be available on the PATH, if python3 is available, using `python3` there is the **right** thing to do .
--
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] Melodie97 commented on a change in pull request #21346: Update docs/readme.rst
Posted by GitBox <gi...@apache.org>.
Melodie97 commented on a change in pull request #21346:
URL: https://github.com/apache/airflow/pull/21346#discussion_r801582538
##########
File path: docs/start_doc_server.sh
##########
@@ -20,5 +20,11 @@ DOCS_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"
readonly DOCS_DIR
(cd "${DOCS_DIR}"/_build || exit;
- python -m http.server 8000
+ version=$(python -V 2>&1 | grep -Po '(?<=Python )(.+)')
+ if [[ -z "$version" ]]
+ then
+ python3 -m http.server 8000
Review comment:
@potiuk ok I should just replace the python with python3?
--
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 #21346: Update docs/readme.rst
Posted by GitBox <gi...@apache.org>.
potiuk merged pull request #21346:
URL: https://github.com/apache/airflow/pull/21346
--
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 change in pull request #21346: Update docs/readme.rst
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #21346:
URL: https://github.com/apache/airflow/pull/21346#discussion_r801661381
##########
File path: docs/start_doc_server.sh
##########
@@ -20,5 +20,11 @@ DOCS_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"
readonly DOCS_DIR
(cd "${DOCS_DIR}"/_build || exit;
- python -m http.server 8000
+ version=$(python -V 2>&1 | grep -Po '(?<=Python )(.+)')
+ if [[ -z "$version" ]]
+ then
+ python3 -m http.server 8000
Review comment:
yep.
--
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 #21346: Update docs/readme.rst
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #21346:
URL: https://github.com/apache/airflow/pull/21346#issuecomment-1035083840
Just rebase. Good way to try the new "Update with rebase" feature of GitHub that makes it sooooo much easier:
--
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] Melodie97 commented on a change in pull request #21346: Update docs/readme.rst
Posted by GitBox <gi...@apache.org>.
Melodie97 commented on a change in pull request #21346:
URL: https://github.com/apache/airflow/pull/21346#discussion_r801001745
##########
File path: docs/README.rst
##########
@@ -74,7 +74,24 @@ Once you have built the documentation run the following command from the root di
docs/start_doc_server.sh
-Then, view your docs at ``localhost:8000``
+You need to have python installed to run the command, if you get an error message -- python: command not found
+It is possible that you don't have python installed or added to PATH.
+Visit link <https://python.land/installing-python> to install python and add to PATH
Review comment:
ok
--
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] Melodie97 commented on pull request #21346: Update docs/readme.rst
Posted by GitBox <gi...@apache.org>.
Melodie97 commented on pull request #21346:
URL: https://github.com/apache/airflow/pull/21346#issuecomment-1034960078
@potiuk It's showing that static checks are failing here but it didn't show this in my local environment after I used pre-commit, also the error description says this has something to do with run mypy
--
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] Melodie97 commented on pull request #21346: Update docs/readme.rst
Posted by GitBox <gi...@apache.org>.
Melodie97 commented on pull request #21346:
URL: https://github.com/apache/airflow/pull/21346#issuecomment-1032379925
@potiuk , pls review
--
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 change in pull request #21346: Update docs/readme.rst
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #21346:
URL: https://github.com/apache/airflow/pull/21346#discussion_r801556998
##########
File path: docs/start_doc_server.sh
##########
@@ -20,5 +20,11 @@ DOCS_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"
readonly DOCS_DIR
(cd "${DOCS_DIR}"/_build || exit;
- python -m http.server 8000
+ version=$(python -V 2>&1 | grep -Po '(?<=Python )(.+)')
+ if [[ -z "$version" ]]
+ then
+ python3 -m http.server 8000
Review comment:
I think we do not need to check the version at all. Simply using python3 wil be enough, without the need of checking the version.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr commented on a change in pull request #21346: Update docs/readme.rst
Posted by GitBox <gi...@apache.org>.
uranusjr commented on a change in pull request #21346:
URL: https://github.com/apache/airflow/pull/21346#discussion_r800176139
##########
File path: docs/README.rst
##########
@@ -74,7 +74,24 @@ Once you have built the documentation run the following command from the root di
docs/start_doc_server.sh
-Then, view your docs at ``localhost:8000``
+You need to have python installed to run the command, if you get an error message -- python: command not found
+It is possible that you don't have python installed or added to PATH.
+Visit link <https://python.land/installing-python> to install python and add to PATH
Review comment:
I think the `--` is not a part of the message? It should be
```rst
if you get an error message -- ``python: command not found``
```
instead.
Also please capitalise *python* except in code blocks.
--
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