Skip to content

Commit

Permalink
Fix selective checks to work for non-main-branch (#24769)
Browse files Browse the repository at this point in the history
The recent changes in selective checks introduced a few problems
in non-main branches:

* constraints branch was not used by CI build in non-main branch
  this was not a proble, (until it started to fail) because
  other branches always work in "upgradeDepencies" case

* default branch calculation did not account for the selective-check
  comment that was run before default branch was retrieved. Therefore
  we did not run build as from 2.3 branch

* Some precomits should be skiped in non-main branch

* Integration was also excluded from Provider Check

* Two more commit outputs added:
  * docs-filter -  depends whether you are with more packages
  * skip-pre-commit - allows to dynamically choose which pre-commits
    should be skipped
  • Loading branch information
potiuk authored Jul 1, 2022
1 parent f18c609 commit c8d3850
Show file tree
Hide file tree
Showing 7 changed files with 76 additions and 66 deletions.
87 changes: 32 additions & 55 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,8 @@ jobs:
GITHUB_CONTEXT: ${{ toJson(github) }}
outputs:
defaultBranch: ${{ steps.selective-checks.outputs.default-branch }}
defaultConstraintsBranch: ${{ steps.selective-checks.outputs.default-constraints-branch }}
debianVersion: ${{ steps.selective-checks.outputs.debian-version }}
cacheDirective: ${{ steps.dynamic-outputs.outputs.cacheDirective }}
waitForImage: ${{ steps.wait-for-image.outputs.wait-for-image }}
allPythonVersions: ${{ steps.selective-checks.outputs.all-python-versions }}
Expand Down Expand Up @@ -164,6 +166,8 @@ jobs:
needs-api-tests: ${{ steps.selective-checks.outputs.needs-api-tests }}
needs-api-codegen: ${{ steps.selective-checks.outputs.needs-api-codegen }}
default-branch: ${{ steps.selective-checks.outputs.default-branch }}
docs-filter: ${{ steps.selective-checks.outputs.docs-filter }}
skip-pre-commits: ${{ steps.selective-checks.outputs.skip-pre-commits }}
sourceHeadRepo: ${{ steps.source-run-info.outputs.sourceHeadRepo }}
pullRequestNumber: ${{ steps.source-run-info.outputs.pullRequestNumber }}
pullRequestLabels: ${{ steps.source-run-info.outputs.pullRequestLabels }}
Expand Down Expand Up @@ -191,7 +195,6 @@ jobs:
ref: ${{ github.sha }}
fetch-depth: 2
persist-credentials: false
if: github.event_name == 'pull_request'
- name: "Setup python"
uses: actions/setup-python@v4
with:
Expand All @@ -200,6 +203,22 @@ jobs:
cache: 'pip'
cache-dependency-path: ./dev/breeze/setup*
- run: ./scripts/ci/install_breeze.sh
- name: "Retrieve DEFAULTS from the _initialization.sh"
# We cannot "source" the script here because that would be a security problem (we cannot run
# any code that comes from the sources coming from the PR. Therefore, we extract the
# DEFAULT_BRANCH and DEFAULT_CONSTRAINTS_BRANCH and DEBIAN_VERSION via custom grep/awk/sed commands
id: defaults
run: |
DEFAULT_BRANCH=$(grep "export DEFAULT_BRANCH" scripts/ci/libraries/_initialization.sh | \
awk 'BEGIN{FS="="} {print $3}' | sed s'/["}]//g')
echo "DEFAULT_BRANCH=${DEFAULT_BRANCH}" >> $GITHUB_ENV
DEFAULT_CONSTRAINTS_BRANCH=$(grep "export DEFAULT_CONSTRAINTS_BRANCH" \
scripts/ci/libraries/_initialization.sh | \
awk 'BEGIN{FS="="} {print $3}' | sed s'/["}]//g')
echo "DEFAULT_CONSTRAINTS_BRANCH=${DEFAULT_CONSTRAINTS_BRANCH}" >> $GITHUB_ENV
DEBIAN_VERSION=$(grep "export DEBIAN_VERSION" scripts/ci/libraries/_initialization.sh | \
awk 'BEGIN{FS="="} {print $3}' | sed s'/["}]//g')
echo "DEBIAN_VERSION=${DEBIAN_VERSION}" >> $GITHUB_ENV
- name: Selective checks
id: selective-checks
env:
Expand Down Expand Up @@ -294,6 +313,9 @@ jobs:
runs-on: ${{ fromJson(needs.build-info.outputs.runsOn) }}
needs: [build-info]
env:
DEFAULT_BRANCH: ${{ needs.build-info.outputs.default-branch }}
DEFAULT_CONSTRAINTS_BRANCH: ${{ needs.build-info.outputs.default-constraints-branch }}
DEBIAN_VERSION: ${{ needs.build-info.outputs.debian-version }}
RUNS_ON: ${{ fromJson(needs.build-info.outputs.runsOn)[0] }}
steps:
- name: Cleanup repo
Expand All @@ -310,23 +332,6 @@ jobs:
with:
python-version: ${{ needs.build-info.outputs.defaultPythonVersion }}
if: needs.build-info.outputs.inWorkflowBuild == 'true'
- name: "Retrieve DEFAULTS from the _initialization.sh"
# We cannot "source" the script here because that would be a security problem (we cannot run
# any code that comes from the sources coming from the PR. Therefore, we extract the
# DEFAULT_BRANCH and DEFAULT_CONSTRAINTS_BRANCH and DEBIAN_VERSION via custom grep/awk/sed commands
id: defaults
run: |
DEFAULT_BRANCH=$(grep "export DEFAULT_BRANCH" scripts/ci/libraries/_initialization.sh | \
awk 'BEGIN{FS="="} {print $3}' | sed s'/["}]//g')
echo "DEFAULT_BRANCH=${DEFAULT_BRANCH}" >> $GITHUB_ENV
DEFAULT_CONSTRAINTS_BRANCH=$(grep "export DEFAULT_CONSTRAINTS_BRANCH" \
scripts/ci/libraries/_initialization.sh | \
awk 'BEGIN{FS="="} {print $3}' | sed s'/["}]//g')
echo "DEFAULT_CONSTRAINTS_BRANCH=${DEFAULT_CONSTRAINTS_BRANCH}" >> $GITHUB_ENV
DEBIAN_VERSION=$(grep "export DEBIAN_VERSION" scripts/ci/libraries/_initialization.sh | \
awk 'BEGIN{FS="="} {print $3}' | sed s'/["}]//g')
echo "DEBIAN_VERSION=${DEBIAN_VERSION}" >> $GITHUB_ENV
if: needs.build-info.outputs.inWorkflowBuild == 'true'
- run: ./scripts/ci/install_breeze.sh
if: needs.build-info.outputs.inWorkflowBuild == 'true'
- name: "Free space"
Expand Down Expand Up @@ -361,6 +366,9 @@ jobs:
runs-on: ${{ fromJson(needs.build-info.outputs.runsOn) }}
needs: [build-info, build-ci-images]
env:
DEFAULT_BRANCH: ${{ needs.build-info.outputs.default-branch }}
DEFAULT_CONSTRAINTS_BRANCH: ${{ needs.build-info.outputs.default-constraints-branch }}
DEBIAN_VERSION: ${{ needs.build-info.outputs.debian-version }}
RUNS_ON: ${{ fromJson(needs.build-info.outputs.runsOn)[0] }}
BACKEND: sqlite
DOCKER_CACHE: ${{ needs.build-info.outputs.cacheDirective }}
Expand All @@ -380,23 +388,6 @@ jobs:
with:
python-version: ${{ needs.build-info.outputs.defaultPythonVersion }}
if: needs.build-info.outputs.inWorkflowBuild == 'true'
- name: "Retrieve DEFAULTS from the _initialization.sh"
# We cannot "source" the script here because that would be a security problem (we cannot run
# any code that comes from the sources coming from the PR. Therefore we extract the
# DEFAULT_BRANCH and DEFAULT_CONSTRAINTS_BRANCH and DEBIAN_VERSION via custom grep/awk/sed commands
id: defaults
run: |
DEFAULT_BRANCH=$(grep "export DEFAULT_BRANCH" scripts/ci/libraries/_initialization.sh | \
awk 'BEGIN{FS="="} {print $3}' | sed s'/["}]//g')
echo "DEFAULT_BRANCH=${DEFAULT_BRANCH}" >> $GITHUB_ENV
DEFAULT_CONSTRAINTS_BRANCH=$(grep "export DEFAULT_CONSTRAINTS_BRANCH" \
scripts/ci/libraries/_initialization.sh | \
awk 'BEGIN{FS="="} {print $3}' | sed s'/["}]//g')
echo "DEFAULT_CONSTRAINTS_BRANCH=${DEFAULT_CONSTRAINTS_BRANCH}" >> $GITHUB_ENV
DEBIAN_VERSION=$(grep "export DEBIAN_VERSION" scripts/ci/libraries/_initialization.sh | \
awk 'BEGIN{FS="="} {print $3}' | sed s'/["}]//g')
echo "DEBIAN_VERSION=${DEBIAN_VERSION}" >> $GITHUB_ENV
if: needs.build-info.outputs.inWorkflowBuild == 'true'
- run: ./scripts/ci/install_breeze.sh
if: needs.build-info.outputs.inWorkflowBuild == 'true'
- name: "Free space"
Expand Down Expand Up @@ -644,7 +635,7 @@ ${{ hashFiles('.pre-commit-config.yaml') }}"
run: breeze static-checks --all-files --show-diff-on-failure --color always
env:
VERBOSE: "false"
SKIP: "identity"
SKIP: ${{ needs.build-info.outputs.skip-pre-commits }}
COLUMNS: "250"
- name: "Fix ownership"
run: breeze fix-ownership
Expand Down Expand Up @@ -701,7 +692,7 @@ ${{ hashFiles('.pre-commit-config.yaml') }}"
env:
VERBOSE: "false"
SKIP_IMAGE_PRE_COMMITS: "true"
SKIP: "identity"
SKIP: ${{ needs.build-info.outputs.skip-pre-commits }}
COLUMNS: "250"
- name: "Fix ownership"
run: breeze fix-ownership
Expand Down Expand Up @@ -745,7 +736,7 @@ ${{ hashFiles('.pre-commit-config.yaml') }}"
docs-inventory-${{ hashFiles('setup.py','setup.cfg','pyproject.toml;') }}
docs-inventory-
- name: "Build docs"
run: breeze build-docs
run: breeze build-docs ${{ needs.build-info.outputs.docs-filter }}
- name: Configure AWS credentials
uses: ./.github/actions/configure-aws-credentials
if: >
Expand Down Expand Up @@ -1766,6 +1757,9 @@ ${{ hashFiles('.pre-commit-config.yaml') }}"
- tests-mssql
- tests-postgres
env:
DEFAULT_BRANCH: ${{ needs.build-info.outputs.default-branch }}
DEFAULT_CONSTRAINTS_BRANCH: ${{ needs.build-info.outputs.default-constraints-branch }}
DEBIAN_VERSION: ${{ needs.build-info.outputs.debian-version }}
RUNS_ON: ${{ fromJson(needs.build-info.outputs.runsOn)[0] }}
if: needs.build-info.outputs.upgradeToNewerDependencies != 'false'
steps:
Expand All @@ -1783,23 +1777,6 @@ ${{ hashFiles('.pre-commit-config.yaml') }}"
with:
python-version: ${{ needs.build-info.outputs.defaultPythonVersion }}
if: needs.build-info.outputs.inWorkflowBuild == 'true'
- name: "Retrieve DEFAULTS from the _initialization.sh"
# We cannot "source" the script here because that would be a security problem (we cannot run
# any code that comes from the sources coming from the PR. Therefore, we extract the
# DEFAULT_BRANCH and DEFAULT_CONSTRAINTS_BRANCH and DEBIAN_VERSION via custom grep/awk/sed commands
id: defaults
run: |
DEFAULT_BRANCH=$(grep "export DEFAULT_BRANCH" scripts/ci/libraries/_initialization.sh | \
awk 'BEGIN{FS="="} {print $3}' | sed s'/["}]//g')
echo "DEFAULT_BRANCH=${DEFAULT_BRANCH}" >> $GITHUB_ENV
DEFAULT_CONSTRAINTS_BRANCH=$(grep "export DEFAULT_CONSTRAINTS_BRANCH" \
scripts/ci/libraries/_initialization.sh | \
awk 'BEGIN{FS="="} {print $3}' | sed s'/["}]//g')
echo "DEFAULT_CONSTRAINTS_BRANCH=${DEFAULT_CONSTRAINTS_BRANCH}" >> $GITHUB_ENV
DEBIAN_VERSION=$(grep "export DEBIAN_VERSION" scripts/ci/libraries/_initialization.sh | \
awk 'BEGIN{FS="="} {print $3}' | sed s'/["}]//g')
echo "DEBIAN_VERSION=${DEBIAN_VERSION}" >> $GITHUB_ENV
if: needs.build-info.outputs.inWorkflowBuild == 'true'
- run: ./scripts/ci/install_breeze.sh
if: needs.build-info.outputs.inWorkflowBuild == 'true'
- name: "Free space"
Expand Down
4 changes: 4 additions & 0 deletions dev/breeze/src/airflow_breeze/params/doc_build_params.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@
from dataclasses import dataclass
from typing import List, Tuple

from airflow_breeze.branch_defaults import AIRFLOW_BRANCH


@dataclass
class DocBuildParams:
Expand All @@ -38,6 +40,8 @@ def args_doc_builder(self) -> List[str]:
doc_args.append("--spellcheck-only")
if self.for_production:
doc_args.append("--for-production")
if AIRFLOW_BRANCH != "main":
doc_args.append("--disable-provider-checks")
if self.package_filter and len(self.package_filter) > 0:
for single_filter in self.package_filter:
doc_args.extend(["--package-filter", single_filter])
Expand Down
18 changes: 18 additions & 0 deletions dev/breeze/src/airflow_breeze/utils/selective_checks.py
Original file line number Diff line number Diff line change
Expand Up @@ -469,6 +469,12 @@ def test_types(self) -> str:
f"is {self._default_branch} and not main[/]"
)
current_test_types.remove("Providers")
if "Integration" in current_test_types:
get_console().print(
"[warning]Removing 'Integration' because the target branch "
f"is {self._default_branch} and not main[/]"
)
current_test_types.remove("Integration")
return " ".join(sorted(current_test_types))

@cached_property
Expand All @@ -480,3 +486,15 @@ def upgrade_to_newer_dependencies(self) -> bool:
return len(
self._matching_files(FileGroupForCi.SETUP_FILES, CI_FILE_GROUP_MATCHES)
) > 0 or self._github_event in [GithubEvents.PUSH, GithubEvents.SCHEDULE]

@cached_property
def docs_filter(self) -> str:
return (
""
if self._default_branch == 'main'
else "--package-filter apache-airflow --package-filter docker-stack"
)

@cached_property
def skip_pre_commits(self) -> str:
return "identity" if self._default_branch == "main" else "identity,check-airflow-2-2-compatibility"
14 changes: 8 additions & 6 deletions dev/breeze/tests/test_selective_checks.py
Original file line number Diff line number Diff line change
Expand Up @@ -275,9 +275,10 @@ def test_expected_output_pull_request_main(
"run-tests": "true",
"docs-build": "true",
"upgrade-to-newer-dependencies": "false",
"test-types": "API Always CLI Core Integration Other WWW",
"test-types": "API Always CLI Core Other WWW",
},
id="Everything should run except Providers when full tests are needed for non-main branch",
id="Everything should run except Providers and Integration "
"when full tests are needed for non-main branch",
)
),
],
Expand Down Expand Up @@ -367,9 +368,10 @@ def test_expected_output_full_tests_needed(
"docs-build": "true",
"run-kubernetes-tests": "false",
"upgrade-to-newer-dependencies": "false",
"test-types": "API Always CLI Core Integration Other WWW",
"test-types": "API Always CLI Core Other WWW",
},
id="All tests except providers should run if core file changed in non-main branch",
id="All tests except providers and Integration should "
"run if core file changed in non-main branch",
),
],
)
Expand Down Expand Up @@ -488,9 +490,9 @@ def test_expected_output_pull_request_target(
"run-tests": "true",
"docs-build": "true",
"upgrade-to-newer-dependencies": "true",
"test-types": "API Always CLI Core Integration Other WWW",
"test-types": "API Always CLI Core Other WWW",
},
id="All tests except Providers and Helm run on push"
id="All tests except Providers Integration and Helm run on push"
" even if unimportant file changed in non-main branch",
),
pytest.param(
Expand Down
9 changes: 8 additions & 1 deletion docs/build_docs.py
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,12 @@ def _get_parser():
parser.add_argument(
'--disable-checks', dest='disable_checks', action='store_true', help='Disables extra checks'
)
parser.add_argument(
'--disable-provider-checks',
dest='disable_provider_checks',
action='store_true',
help='Disables extra checks for providers',
)
parser.add_argument(
'--one-pass-only',
dest='one_pass_only',
Expand Down Expand Up @@ -441,6 +447,7 @@ def main():
available_packages = get_available_packages()
docs_only = args.docs_only
spellcheck_only = args.spellcheck_only
disable_provider_checks = args.disable_provider_checks
disable_checks = args.disable_checks
package_filters = args.package_filter
for_production = args.for_production
Expand Down Expand Up @@ -531,7 +538,7 @@ def main():
)

if not disable_checks:
general_errors = lint_checks.run_all_check()
general_errors = lint_checks.run_all_check(disable_provider_checks=disable_provider_checks)
if general_errors:
all_build_errors[None] = general_errors

Expand Down
2 changes: 2 additions & 0 deletions docs/conf.py
Original file line number Diff line number Diff line change
Expand Up @@ -694,6 +694,8 @@ def _get_params(root_schema: dict, prefix: str = "", default_section: str = "")
]
if PACKAGE_NAME == 'apache-airflow':
autoapi_ignore.append('*/airflow/providers/*')
elif PACKAGE_NAME == 'docker-stack':
autoapi_ignore.append('*/airflow/providers/*')
else:
autoapi_ignore.append('*/airflow/providers/cncf/kubernetes/backcompat/*')
# Keep the AutoAPI generated files on the filesystem after the run.
Expand Down
8 changes: 4 additions & 4 deletions docs/exts/docs_build/lint_checks.py
Original file line number Diff line number Diff line change
Expand Up @@ -323,13 +323,13 @@ def check_pypi_repository_in_provider_tocs() -> List[DocBuildError]:
return build_errors


def run_all_check() -> List[DocBuildError]:
def run_all_check(disable_provider_checks: bool = False) -> List[DocBuildError]:
"""Run all checks from this module"""
general_errors = []
general_errors.extend(check_guide_links_in_operator_descriptions())
general_errors.extend(check_enforce_code_block())
general_errors.extend(check_exampleinclude_for_example_dags())
general_errors.extend(check_example_dags_in_provider_tocs())
general_errors.extend(check_pypi_repository_in_provider_tocs())

if not disable_provider_checks:
general_errors.extend(check_pypi_repository_in_provider_tocs())
general_errors.extend(check_example_dags_in_provider_tocs())
return general_errors

0 comments on commit c8d3850

Please sign in to comment.