Airflow: Adapt better variable naming conventions in our bash scripts

[potiuk]

Description

Currently, in our bash scripts (mostly in CI and Breeze) we use an old and outdated naming convention where every environment variable is CAPITALIZED. This comes from my personal background as I am the author of most of it, but there are better (and modern) conventions that we should use. Particularly https://google.github.io/styleguide/shellguide.html is one that is interesting but it has a number of google-internal-specific decisions that might not apply to our case.

The other shell style gule that seems tobe much more reasonable/open-sorce friendly and widely quoted is the "icy" one https://github.com/icy/bash-coding-style.

I think it's good to apply the naming convention of Google - where we should use:

• CAPITALIZATION to indicate constant and exported variables
• readonly for read-only values (especially those that are read from configuration and remain consistent across the rest of the script
• local variables

Use case / motivation

Make the bash scripts more readable and robust, avoid duplication of code and allows easier future maintenance

Bash scripts to review

• [x] ./breeze
• [x] ./breeze-complete
• [x] ./docs/start_doc_server.sh
• [x] ./chart/dockerfiles/pgbouncer/build_and_push.sh
• [x] ./chart/dockerfiles/statsd-exporter/build_and_push.sh
• [x] ./dev/sign.sh
• [x] ./airflow/www/compile_assets.sh
• [ ] ./scripts/in_container/_in_container_script_init.sh
• [ ] ./scripts/in_container/run_mypy.sh
• [ ] ./scripts/in_container/run_docs_build.sh
• [ ] ./scripts/in_container/refresh_pylint_todo.sh
• [ ] ./scripts/in_container/_in_container_utils.sh
• [ ] ./scripts/in_container/run_prepare_backport_packages.sh
• [ ] ./scripts/in_container/run_generate_constraints.sh
• [ ] ./scripts/in_container/run_clear_tmp.sh
• [ ] ./scripts/in_container/run_ci_tests.sh
• [ ] ./scripts/in_container/run_test_package_installation_separately.sh
• [ ] ./scripts/in_container/run_extract_tests.sh
• [ ] ./scripts/in_container/run_prepare_backport_readme.sh
• [ ] ./scripts/in_container/run_flake8.sh
• [ ] ./scripts/in_container/run_cli_tool.sh
• [ ] ./scripts/in_container/run_system_tests.sh
• [ ] ./scripts/in_container/entrypoint_exec.sh
• [ ] ./scripts/in_container/run_pylint.sh
• [ ] ./scripts/in_container/run_test_package_import_all_classes.sh
• [ ] ./scripts/in_container/run_fix_ownership.sh
• [ ] ./scripts/in_container/entrypoint_ci.sh
• [ ] ./scripts/in_container/configure_environment.sh
• [ ] ./scripts/in_container/check_environment.sh
• [ ] ./scripts/in_container/prod/airflow_scheduler_autorestart.sh
• [ ] ./scripts/in_container/prod/entrypoint_prod.sh
• [ ] ./scripts/in_container/prod/clean-logs.sh
• [x] ./scripts/ci/docs/ci_docs.sh
• [x] ./scripts/ci/testing/ci_run_airflow_testing.sh
• [x] ./scripts/ci/kubernetes/ci_deploy_app_to_kubernetes.sh
• [x] ./scripts/ci/kubernetes/ci_run_kubernetes_tests.sh
• [x] ./scripts/ci/kubernetes/ci_run_helm_testing.sh
• [ ] ./scripts/ci/libraries/_pylint.sh
• [ ] ./scripts/ci/libraries/_permissions.sh
• [ ] ./scripts/ci/libraries/_spinner.sh
• [ ] ./scripts/ci/libraries/_kind.sh
• [ ] ./scripts/ci/libraries/_md5sum.sh
• [ ] ./scripts/ci/libraries/_sanity_checks.sh
• [ ] ./scripts/ci/libraries/_verbosity.sh
• [ ] ./scripts/ci/libraries/_parameters.sh
• [ ] ./scripts/ci/libraries/_initialization.sh
• [ ] ./scripts/ci/libraries/_script_init.sh
• [ ] ./scripts/ci/libraries/_push_pull_remove_images.sh
• [ ] ./scripts/ci/libraries/_start_end.sh
• [ ] ./scripts/ci/libraries/_build_images.sh
• [ ] ./scripts/ci/libraries/_runs.sh
• [ ] ./scripts/ci/libraries/_local_mounts.sh
• [ ] ./scripts/ci/libraries/_all_libs.sh
• [x] ./scripts/ci/constraints/ci_commit_constraints.sh
• [x] ./scripts/ci/constraints/ci_generate_constraints.sh
• [x] ./scripts/ci/constraints/ci_branch_constraints.sh
• [x] ./scripts/ci/static_checks/refresh_pylint_todo.sh
• [x] ./scripts/ci/static_checks/mypy.sh
• [x] ./scripts/ci/static_checks/bat_tests.sh
• [x] ./scripts/ci/static_checks/pylint.sh
• [x] ./scripts/ci/static_checks/run_static_checks.sh
• [x] ./scripts/ci/static_checks/lint_dockerfile.sh
• [x] ./scripts/ci/static_checks/flake8.sh
• [x] ./scripts/ci/static_checks/check_license.sh
• [x] ./scripts/ci/openapi/client_codegen_diff.sh
• [x] ./scripts/ci/pre_commit/pre_commit_mypy.sh
• [x] ./scripts/ci/pre_commit/pre_commit_local_yml_mounts.sh
• [x] ./scripts/ci/pre_commit/pre_commit_lint_dockerfile.sh
• [x] ./scripts/ci/pre_commit/pre_commit_ci_build.sh
• [x] ./scripts/ci/pre_commit/pre_commit_setup_cfg_file.sh
• [x] ./scripts/ci/pre_commit/pre_commit_flake8.sh
• [x] ./scripts/ci/pre_commit/pre_commit_build_providers_dependencies.sh
• [x] ./scripts/ci/pre_commit/pre_commit_check_license.sh
• [x] ./scripts/ci/pre_commit/pre_commit_pylint.sh
• [x] ./scripts/ci/pre_commit/pre_commit_check_integrations.sh
• [x] ./scripts/ci/pre_commit/pre_commit_mermaid.sh
• [x] ./scripts/ci/pre_commit/pre_commit_bat_tests.sh
• [x] ./scripts/ci/pre_commit/pre_commit_breeze_cmd_line.sh
• [x] ./scripts/ci/tools/ci_count_changed_files.sh
• [x] ./scripts/ci/tools/ci_clear_tmp.sh
• [x] ./scripts/ci/tools/ci_free_space_on_ci.sh
• [x] ./scripts/ci/tools/ci_fix_ownership.sh
• [x] ./scripts/ci/tools/ci_check_if_tests_should_be_run.sh
• [x] ./scripts/ci/backport_packages/ci_test_backport_packages_import_all_classes.sh
• [x] ./scripts/ci/backport_packages/ci_prepare_backport_packages.sh
• [x] ./scripts/ci/backport_packages/ci_prepare_backport_readme.sh
• [x] ./scripts/ci/backport_packages/ci_test_backport_packages_install_separately.sh
• [x] ./scripts/ci/backport_packages/ci_prepare_and_test_backport_packages.sh
• [x] ./scripts/ci/images/ci_prepare_ci_image_on_ci.sh
• [x] ./scripts/ci/images/ci_build_dockerhub.sh
• [x] ./scripts/ci/images/ci_prepare_prod_image_on_ci.sh
• [x] ./scripts/ci/images/ci_wait_for_all_prod_images.sh
• [x] ./scripts/ci/images/ci_push_production_images.sh
• [x] ./scripts/ci/images/ci_wait_for_all_ci_images.sh
• [x] ./scripts/ci/images/ci_push_ci_images.sh
• [x] ./backport_packages/build_source_package.sh
• [x] ./images/breeze/add_overlay.sh
• [x] ./clients/gen/common.sh
• [x] ./clients/gen/go.sh
• [x] ./tests/bats/mocks/docker.sh
• [x] ./tests/bats/mocks/kubectl.sh
• [x] ./tests/bats/mocks/kind.sh
• [x] ./tests/bats/mocks/helm.sh

[OmairK]

@potiuk is this issue up for taking? I would like to work on it.

[potiuk]

Sure @OmairK. Feel free :) Ideally one-by-one :)

[potiuk]

I am about to submit a big Breeze change for that, so you can take a look how I 've done that and review :) @OmairK

[OmairK]

I am about to submit a big Breeze change for that, so you can take a look how I 've done that and review :) @OmairK

Yes that would be very helpful thanks :smile:

[potiuk]

Hey @OmairK (and anyone else) -> feel free to pick any of the files from the list. some of them are very small and super easy. The #10651 contains detailed description (in commit message) of the changes implemented in Breeze and related scripts and the "context" of what we want to achieve with applying the guidelines.

[werbolis]

Hi @potiuk, I'd like to help with this issue, would it be OK to take in_container_utils and related stuff?

[OmairK]

Hi @potiuk, I'd like to help with this issue, would it be OK to take in_container_utils and related stuff?

Hey @werbolis I had a WIP PR for in_container_utils, due to some personal commitments I have'nt been able to work on it lately let me know if you would like to co-author this PR.

[werbolis]

Hi @OmairK sure, why not. Should I fork your repo and make a PR to your branch, or is there some better way? Sorry if that's obvious, it's my first contribution attempt :)

[OmairK]

Hi @OmairK sure, why not. Should I fork your repo and make a PR to your branch, or is there some better way? Sorry if that's obvious, it's my first contribution attempt :)

@werbolis I have added you as a collaborator to my fork. You could directly contribute to the master branch and the changes that you push will be reflected on this PR (sorry mixed up the WIP PR links in the last comment). Let me know if you have any trouble, I will be happy to help :smile:.

[OliverWBurke]

i would like to pick up some of these

[potiuk]

Feel free :)

[tahakashaf]

I'd like to work on some of these

[werbolis]

@OliverWBurke @tahakashaf Just a note to prevent doubling work :) I'm working on a PR started by @OmairK, related to in_container scripts. It's work in progress, but somewhat advanced; if you'd like to work on them, maybe you could help us with review/ideas? The PR does not touch libraries.