[GitHub] [airflow] potiuk commented on a change in pull request #20886: build documentation breeze

[GitBox <gi...@apache.org>]

potiuk commented on a change in pull request #20886:
URL: https://github.com/apache/airflow/pull/20886#discussion_r785407059



##########
File path: dev/breeze/src/airflow_breeze/breeze.py
##########
@@ -276,6 +283,17 @@ def change_config(python, backend, cheatsheet, asciiart):
         console.print(f'[blue]Backend cached_value {backend}')
 
 
+@option_verbose
+@main.command(name='build-docs')
+@click.option('--docs-only', is_flag=True)
+@click.option('--spellcheck-only', is_flag=True)
+@click.option('--package-filter', type=click.Choice(AVAILABLE_PACKAGES))

Review comment:
       The option here should be "multiple" https://click.palletsprojects.com/en/8.0.x/options/#multiple-options. You should be able to specify:
   
   --package-filter docker-stat --package-filter apache-airflow --package-filter apache-airflow-provider-google.
   
   That will turn the "package_filter" into `List[str]`

##########
File path: dev/breeze/src/airflow_breeze/global_constants.py
##########
@@ -188,3 +188,81 @@
 MSSQL_HOST_PORT = "21433"
 FLOWER_HOST_PORT = "25555"
 REDIS_HOST_PORT = "26379"
+
+AVAILABLE_PACKAGES = [

Review comment:
       This list should be generated from the directories available in `docs`  (minus few folders that we know we should skip: "exts", "integration-logos", "rtd-deprecation". It will change in the future and new packages will be added so it would be great to get this list automatically (it should be fast for click as well).

##########
File path: dev/breeze/src/airflow_breeze/breeze.py
##########
@@ -276,6 +283,17 @@ def change_config(python, backend, cheatsheet, asciiart):
         console.print(f'[blue]Backend cached_value {backend}')
 
 
+@option_verbose
+@main.command(name='build-docs')
+@click.option('--docs-only', is_flag=True)
+@click.option('--spellcheck-only', is_flag=True)
+@click.option('--package-filter', type=click.Choice(AVAILABLE_PACKAGES))

Review comment:
       There is a small problem with the choices though as they allow also some other values. Package-filter can take values like `apache-airflow-provider-*`. And Click has no support for "list" autocomplete with some additional options available. Though I think this is arguable if it is useful or not. I don't think I ever used it to be honest. 
   
   I am inclined to skip the 'wildcard' capability and leave the list of packages only here.
   
   WDYT @mik-laj ? FYI: this is the wrapper around the "build_docs.py" - with much simpler interface, executing the build always inside the breeze container and autocomplete in Breeze so having fixed set of packages might make sense actually. Maybe we can use `--package` instead of `--package-filter` to indicate that those are only lists of packages and not "filters" as it is in case of `build-docs.py. I find that most of the 'casual" users would likely only use packages and autocomplete there would be **really** useful. Where we can keep `./build_docs.py` for Power users with filtering etc. 
   
   WDYT @mik-laj 




-- 
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