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/08/18 14:25:04 UTC
[GitHub] [airflow] potiuk opened a new pull request #10377: You can disable spellcheck or documentation when building docs.
potiuk opened a new pull request #10377:
URL: https://github.com/apache/airflow/pull/10377
Extracted from #10368
---
**^ 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] potiuk commented on a change in pull request #10377: Allow running spellcheck or building docs separately
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #10377:
URL: https://github.com/apache/airflow/pull/10377#discussion_r472310228
##########
File path: docs/build_docs.py
##########
@@ -15,8 +15,8 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
Review comment:
Yes. There are several reasons for it:
1) there was a problem when I switched to Docker-build only tests in #10368 . In .dockerignore we are ignoring anything with "build" name at any level as build artifact. Usually generated directories (including one generated by setup.py create directories which are named "build". I could probably filter it out, but I think generally naming anything just "build" is a dangerous one precisely for the reason that various tools might skip or ignore it or delete it.
2) Without the .py extension this file does not get recognized as python file by pre-commit. This is why I had to fix a number of pylint/flake/mypy inside it it because it was not verified before.
3) I think generally speaking running it via buid.py should be discouraged in favour of `./breeze build-doc`. The problem is that you never know if you have all the recent extensions installed. For example if you'd do it after adding spellchecking it would have failed and you would have to figure out that you have to re-rerun 'pip install .[doc]' again. For people who are casual contributors and do it very rarely this might come as a surprise. Also they might not remember that they have a separate virtualenv where they should build the docs and they would loose time trying to find out what's going on. They also might have an even older version of the virtualenv that will not work well.
The`./breeze build-doc` - handles all the venv problems for you and will ask you "rebuild the image" in such case,
which makes it obvious what you should do.
Actually - it's even worse now @kaxil @mik-laj - I just checked that now. And the old instructions are wrong. `pip install -e '.[doc]'` does not work at all:
If you do it from the scratch you get the below error. So unless we know how to fix it, I am removing the old mechanism and replace it with breeze-only. I see no point keeping the instructions which do not work.
```
Module "airflow.providers.amazon.aws.example_dags.example_datasync_1" could not be loaded. Full source will not be available. "error importing 'airflow.providers.amazon.aws.example_dags.example_datasync_1' (exception was: ModuleNotFoundError("No module named 'boto3'",))"
[airflow.providers.amazon.aws.example_dags.example_datasync_1] Entry is false for
[airflow.providers.amazon.aws.example_dags.example_datasync_1] Entry is false for
[airflow.providers.amazon.aws.example_dags.example_datasync_1] Entry is false for
Module "airflow.providers.amazon.aws.example_dags.example_datasync_2" could not be loaded. Full source will not be available. "error importing 'airflow.providers.amazon.aws.example_dags.example_datasync_2' (exception was: ModuleNotFoundError("No module named 'boto3'",))"
[airflow.providers.amazon.aws.example_dags.example_datasync_2] Entry is false for
Module "airflow.providers.amazon.aws.example_dags.example_ecs_fargate" could not be loaded. Full source will not be available. "error importing 'airflow.providers.amazon.aws.example_dags.example_ecs_fargate' (exception was: ModuleNotFoundError("No module named 'boto3'",))"
Module "airflow.providers.amazon.aws.example_dags.example_emr_job_flow_automatic_steps" could not be loaded. Full source will not be available. "error importing 'airflow.providers.amazon.aws.example_dags.example_emr_job_flow_automatic_steps' (exception was: ModuleNotFoundError("No module named 'boto3'",))"
[airflow.providers.amazon.aws.example_dags.example_emr_job_flow_automatic_steps] Entry is false for
Module "airflow.providers.amazon.aws.example_dags.example_emr_job_flow_manual_steps" could not be loaded. Full source will not be available. "error importing 'airflow.providers.amazon.aws.example_dags.example_emr_job_flow_manual_steps' (exception was: ModuleNotFoundError("No module named 'boto3'",))"
Module "airflow.providers.amazon.aws.example_dags.example_google_api_to_s3_transfer_basic" could not be loaded. Full source will not be available. "error importing 'airflow.providers.amazon.aws.example_dags.example_google_api_to_s3_transfer_basic' (exception was: ModuleNotFoundError("No module named 'boto3'",))"
[airflow.providers.amazon.aws.example_dags.example_google_api_to_s3_transfer_basic] Entry is false for
Module "airflow.providers.amazon.aws.example_dags.example_google_api_to_s3_transfer_advanced" could not be loaded. Full source will not be available. "error importing 'airflow.providers.amazon.aws.example_dags.example_google_api_to_s3_transfer_advanced' (exception was: ModuleNotFoundError("No module named 'boto3'",))"
[airflow.providers.amazon.aws.example_dags.example_google_api_to_s3_transfer_advanced] Entry is false for
[airflow.providers.amazon.aws.example_dags.example_google_api_to_s3_transfer_advanced] Entry is false for
[airflow.providers.amazon.aws.example_dags.example_google_api_to_s3_transfer_advanced] Entry is false for
[airflow.providers.amazon.aws.example_dags.example_google_api_to_s3_transfer_advanced] Entry is false for
==================================================
```
----------------------------------------------------------------
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] potiuk commented on a change in pull request #10377: Allow running spellcheck or building docs separately
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #10377:
URL: https://github.com/apache/airflow/pull/10377#discussion_r472310228
##########
File path: docs/build_docs.py
##########
@@ -15,8 +15,8 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
Review comment:
Yes. There are several reasons for it:
1) there was a problem when I switched to Docker-build only tests in #10368 . In .dockerignore we are ignoring anything with "build" name at any level as build artifact. Usually generated directories (including one generated by setup.py create directories which are named "build". I could probably filter it out, but I think generally naming anything just "build" is a dangerous one precisely for the reason that various tools might skip or ignore it or delete it.
2) Without the .py extension this file does not get recognized as python file by pre-commit. This is why I had to fix a number of pylint/flake/mypy inside it it because it was not verified before.
3) I think generally speaking running it via buid.py should be discouraged in favour of `./breeze build-doc`. The problem is that you never know if you have all the recent extensions installed. For example if you'd do it after adding spellchecking it would have failed and you would have to figure out that you have to re-rerun 'pip install .[doc]' again. For people who are casual contributors and do it very rarely this might come as a surprise. Also they might not remember that they have a separate virtualenv where they should build the docs and they would loose time trying to find out what's going on. They also might have an even older version of the virtualenv that will not work well.
The`./breeze build-doc` - handles all the venv problems for you and will ask you "rebuild the image" in such case,
which makes it obvious what you should do.
Actually - it's even worse now @kaxil @mik-laj - I just checked that now. And the old instructions are wrong. `pip install -e '.[doc]'` does not work at all:
If you do it from the scratch you get the below error. So unless we know how to fix it, I am removing the old mechanism and replace it with breeze-only. I see no point keeping the instructions which do not work.
```
Wrong paste. Reproducing 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
[GitHub] [airflow] potiuk commented on a change in pull request #10377: Allow running spellcheck or building docs separately
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #10377:
URL: https://github.com/apache/airflow/pull/10377#discussion_r472316384
##########
File path: docs/build_docs.py
##########
@@ -15,8 +15,8 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
Review comment:
I pasted a wrong (previous output) currently You have a lot of problems like that ^^ unless you have the [al] extras installed . I really think breeze is the only option for contributors to build the docs properly.
----------------------------------------------------------------
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] potiuk commented on pull request #10377: You can disable spellcheck or documentation when building docs.
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #10377:
URL: https://github.com/apache/airflow/pull/10377#issuecomment-675510868
cc: @caddac
----------------------------------------------------------------
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 commented on a change in pull request #10377: You can disable spellcheck or documentation when building docs.
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #10377:
URL: https://github.com/apache/airflow/pull/10377#discussion_r472244534
##########
File path: docs/build_docs.py
##########
@@ -15,8 +15,8 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
Review comment:
LGTM, only question is if we do need to rename the file at all? i.e. just have `docs/build` instead of `docs/build_docs.py`
----------------------------------------------------------------
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] potiuk merged pull request #10377: Allow running spellcheck or building docs separately
Posted by GitBox <gi...@apache.org>.
potiuk merged pull request #10377:
URL: https://github.com/apache/airflow/pull/10377
----------------------------------------------------------------
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] mik-laj commented on a change in pull request #10377: Allow running spellcheck or building docs separately
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #10377:
URL: https://github.com/apache/airflow/pull/10377#discussion_r472582617
##########
File path: docs/build_docs.py
##########
@@ -15,8 +15,8 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
Review comment:
> For example if you'd do it after adding spellchecking it would have failed and you would have to figure out that you have to re-rerun 'pip install .[doc]' again. For people who are casual contributors and do it very rarely this might come as a surprise. Also they might not remember that they have a separate virtualenv where they should build the docs and they would loose time trying to find out what's going on. They also might have an even older version of the virtualenv that will not work well.
This is an issue that occurs with every Python project, so I don't see a big problem here. If you are missing the library, you have to reinstall the dependencies. On other hand, building documentation without Docker is faster.
----------------------------------------------------------------
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] potiuk commented on a change in pull request #10377: Allow running spellcheck or building docs separately
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #10377:
URL: https://github.com/apache/airflow/pull/10377#discussion_r472310228
##########
File path: docs/build_docs.py
##########
@@ -15,8 +15,8 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
Review comment:
Yes. There are several reasons for it:
1) there was a problem when I switched to Docker-build only tests in #10368 . In .dockerignore we are ignoring anything with "build" name at any level as build artifact. Usually generated directories (including one generated by setup.py create directories which are named "build". I could probably filter it out, but I think generally naming anything just "build" is a dangerous one precisely for the reason that various tools might skip or ignore it or delete it.
2) Without the .py extension this file does not get recognized as python file by pre-commit. This is why I had to fix a number of pylint/flake/mypy inside it it because it was not verified before.
3) I think generally speaking running it via buid.py should be discouraged in favour of `./breeze build-doc`. The problem is that you never know if you have all the recent extensions installed. For example if you'd do it after adding spellchecking it would have failed and you would have to figure out that you have to re-rerun 'pip install .[doc]' again. For people who are casual contributors and do it very rarely this might come as a surprise. Also they might not remember that they have a separate virtualenv where they should build the docs and they would loose time trying to find out what's going on. They also might have an even older version of the virtualenv that will not work well.
The`./breeze build-doc` - handles all the venv problems for you and will ask you "rebuild the image" in such case,
which makes it obvious what you should do.
Actually - it's even worse now @kaxil @mik-laj - I just checked that now. And the old instructions are wrong. `pip install -e '.[doc]'` does not work at all:
If you do it from the scratch you get the below error. So unless we know how to fix it, I am removing the old mechanism and replace it with breeze-only. I see no point keeping the instructions which do not work.
```
./docs/build_docs.py
Recreated content of the $/home/jarek/code/airflow/docs/_build and $/home/jarek/code/airflow/docs/_api folders
Executing cmd: sphinx-build -W -b spelling -d _build/doctrees -D extensions=sphinxarg.ext,autoapi.extension,sphinxcontrib.spelling,exampleinclude,sphinx.ext.autodoc,sphinx.ext.coverage,sphinx.ext.viewcode,sphinx.ext.graphviz,sphinxcontrib.httpdomain,sphinxcontrib.jinja,docroles,removemarktransform . _build/spelling
Running Sphinx v3.2.1
Initializing Spelling Checker 5.2.1
/home/jarek/.pyenv/versions/3.6.10/lib/python3.6/importlib/__init__.py:126: RemovedInSphinx40Warning: The alias 'sphinx.ext.autodoc.importer.mock' is deprecated, use 'sphinx.ext.autodoc.mock.mock' instead. Check CHANGES for Sphinx API modifications.
return _bootstrap._gcd_import(name[level:], package, level)
making output directory... failed
Exception occurred:
File "/home/jarek/.pyenv/versions/3.6.10/lib/python3.6/os.py", line 220, in makedirs
mkdir(name, mode)
PermissionError: [Errno 13] Permission denied: '_build/spelling'
The full traceback has been saved in /tmp/sphinx-err-pj53rpvx.log, if you want to report the issue to the developers.
Please also report this if it was a user error, so that a better error message can be provided next time.
A bug report can be filed in the tracker at <https://github.com/sphinx-doc/sphinx/issues>. Thanks!
==================== Error 1 ====================
Sphinx spellcheck returned non-zero exit status: 2.
==================================================
```
----------------------------------------------------------------
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