Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix broken breeze by fixing packaging version #34701

Merged
merged 1 commit into from
Oct 2, 2023

Conversation

hussein-awala
Copy link
Member

The CI is broken in main, and comparing the different runs I noticed that the only difference is the version of packaging where there was a new version 7 hours ago (https://github.com/pypa/packaging/releases/tag/23.2).

@hussein-awala hussein-awala force-pushed the fix_breeze branch 13 times, most recently from 81cff00 to 68624d2 Compare October 2, 2023 07:23
@uranusjr
Copy link
Member

uranusjr commented Oct 2, 2023

Is there log of a failing run? I think the new release has a bug.

@Lee-W
Copy link
Member

Lee-W commented Oct 2, 2023

@uranusjr Not sure whether this is the log you're looking for. But these are the logs for my failing breeze run

on ci, https://github.com/apache/airflow/actions/runs/6377228541/job/17305505217

  File "/opt/pipx_bin/breeze", line 5, in <module>
    from airflow_breeze.breeze import main
  File "/opt/pipx/venvs/apache-airflow-breeze/lib/python3.8/site-packages/airflow_breeze/breeze.py", line 20, in <module>
    from airflow_breeze.commands.main_command import main
  File "/opt/pipx/venvs/apache-airflow-breeze/lib/python3.8/site-packages/airflow_breeze/commands/main_command.py", line 25, in <module>
    from airflow_breeze.commands.ci_image_commands import ci_image
  File "/opt/pipx/venvs/apache-airflow-breeze/lib/python3.8/site-packages/airflow_breeze/commands/ci_image_commands.py", line 31, in <module>
    from airflow_breeze.params.build_ci_params import BuildCiParams
ModuleNotFoundError: No module named 'airflow_breeze.params'

on local

  File "/.../.local/bin/breeze", line 8, in <module>
    sys.exit(main())
  File "/.../.local/pipx/venvs/apache-airflow-breeze/lib/python3.10/site-packages/click/core.py", line 1130, in __call__
    return self.main(*args, **kwargs)
  File "/.../.local/pipx/venvs/apache-airflow-breeze/lib/python3.10/site-packages/rich_click/rich_group.py", line 21, in main
    rv = super().main(*args, standalone_mode=False, **kwargs)
  File "/.../.local/pipx/venvs/apache-airflow-breeze/lib/python3.10/site-packages/click/core.py", line 1055, in main
    rv = self.invoke(ctx)
  File "/.../.local/pipx/venvs/apache-airflow-breeze/lib/python3.10/site-packages/click/core.py", line 1657, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
  File "/.../.local/pipx/venvs/apache-airflow-breeze/lib/python3.10/site-packages/click/core.py", line 1657, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
  File "/.../.local/pipx/venvs/apache-airflow-breeze/lib/python3.10/site-packages/click/core.py", line 1404, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/.../.local/pipx/venvs/apache-airflow-breeze/lib/python3.10/site-packages/click/core.py", line 760, in invoke
    return __callback(*args, **kwargs)
  File "/.../.../airflow/dev/breeze/src/airflow_breeze/commands/ci_image_commands.py", line 310, in build
    run_build(ci_image_params=params)
  File "/.../.../airflow/dev/breeze/src/airflow_breeze/commands/ci_image_commands.py", line 263, in run_build
    return_code, info = run_build_ci_image(
  File "/.../.../airflow/dev/breeze/src/airflow_breeze/commands/ci_image_commands.py", line 577, in run_build_ci_image
    prepare_docker_build_command(
  File "/.../.../airflow/dev/breeze/src/airflow_breeze/utils/docker_command_utils.py", line 491, in prepare_docker_build_command
    build_command = prepare_base_build_command(
  File "/.../.../airflow/dev/breeze/src/airflow_breeze/utils/docker_command_utils.py", line 471, in prepare_base_build_command
    get_and_use_docker_context(image_params.builder),
  File "/.../.../airflow/dev/breeze/src/airflow_breeze/utils/docker_command_utils.py", line 859, in get_and_use_docker_context
    context = autodetect_docker_context()
  File "/.../.../airflow/dev/breeze/src/airflow_breeze/utils/docker_command_utils.py", line 831, in autodetect_docker_context
    context_json = json.loads(result.stdout)
  File "/.../.pyenv/versions/3.10.6/lib/python3.10/json/__init__.py", line 346, in loads
    return _default_decoder.decode(s)
  File "/.../.pyenv/versions/3.10.6/lib/python3.10/json/decoder.py", line 340, in decode
    raise JSONDecodeError("Extra data", s, end)
json.decoder.JSONDecodeError: Extra data: line 2 column 1 (char 149)

@uranusjr
Copy link
Member

uranusjr commented Oct 2, 2023

Hmm that doesn’t seem to be related to packaging. And it’s failing with the same error here too, so maybe there are two separate issues here.

@hussein-awala
Copy link
Member Author

Hmm that doesn’t seem to be related to packaging. And it’s failing with the same error here too, so maybe there are two separate issues here.

Some CI jobs load the scripts files from main (

run: |
rm -rfv "scripts/ci"
mv -v "target-airflow/scripts/ci" "scripts"
rm -rfv "dev"
mv -v "target-airflow/dev" "."
rm -rfv "./github/actions"
mv -v "target-airflow/.github/actions" "actions"
) and for that, these jobs don't consider my change and install the last version of packaging, but if you check Tests / Build info, you see that the change fixes the issue in this job

@uranusjr
Copy link
Member

uranusjr commented Oct 2, 2023

So is neither of the errors above is related to what you’re trying to fix here?

@uranusjr
Copy link
Member

uranusjr commented Oct 2, 2023

OK looks like this is the error

Breeze should only be installed with -e flag

Please go to Airflow sources and run

     breeze self-upgrade --force

which is a consequence of pypa/pipx#1070

I’ve opened pypa/pipx#1071 to fix this so hopefully this will be resolved soon. Since the CI is broken by too many things now I’ll green light to merge this directly.

@hussein-awala
Copy link
Member Author

I will merge to check if the main CI will be fixed.

@hussein-awala hussein-awala marked this pull request as ready for review October 2, 2023 09:21
@hussein-awala hussein-awala merged commit 6618c5f into apache:main Oct 2, 2023
15 of 16 checks passed
@hussein-awala
Copy link
Member Author

Looks like it works 🎉

@uranusjr
Copy link
Member

uranusjr commented Oct 2, 2023

The fix in pipx pypa/pipx#1071 also seems to work for me (breeze ci-image build) so as soon as that gets released we can switch to that instead.

@ephraimbuddy ephraimbuddy added the changelog:skip Changes that should be skipped from the changelog (CI, tests, etc..) label Oct 5, 2023
@ephraimbuddy ephraimbuddy added this to the Airflow 2.7.2 milestone Oct 5, 2023
ephraimbuddy pushed a commit that referenced this pull request Oct 5, 2023
@potiuk
Copy link
Member

potiuk commented Oct 6, 2023

Some CI jobs load the scripts files from main (

@hussein-awala (and others) for the future - the way for committers to test such a PR is to make PR from "airflow" repo, not from a fork. Maintainers can push branches directly to apache/airflow repo and PRs from main are build (including building the image) using the scripts coming from the branch that maintainer pushed to the repo.

That's this flow: https://github.com/apache/airflow/blob/main/CI_DIAGRAMS.md#pull-request-flow-from-apacheairflow-repo

And for the record - this whole setup is done in order to achieve security - users from their fork should not be able to modify code that is run in "Github Actions" - because this code can be harmful and get access to GITHUB_TOKEN, write permissions and other sensitive data (secrets).

This is also one of the requirements by the ASF to enable 'run workflows" automatically for forks for external contributors - otherwise we would have to manually approve every single workflow run.

@Joffreybvn
Copy link
Contributor

Had the same issue today on a fresh Fedora system, installing pipx via brew.

(airflow) [joffreybvn@fedora airflow]$ breeze
Traceback (most recent call last):
  File "/home/joffreybvn/.local/bin/breeze", line 5, in <module>
    from airflow_breeze.breeze import main
  File "/home/joffreybvn/.local/pipx/venvs/apache-airflow-breeze/lib/python3.11/site-packages/airflow_breeze/breeze.py", line 20, in <module>
    from airflow_breeze.commands.main_command import main
  File "/home/joffreybvn/.local/pipx/venvs/apache-airflow-breeze/lib/python3.11/site-packages/airflow_breeze/commands/main_command.py", line 25, in <module>
    from airflow_breeze.commands.ci_image_commands import ci_image
  File "/home/joffreybvn/.local/pipx/venvs/apache-airflow-breeze/lib/python3.11/site-packages/airflow_breeze/commands/ci_image_commands.py", line 31, in <module>
    from airflow_breeze.params.build_ci_params import BuildCiParams
ModuleNotFoundError: No module named 'airflow_breeze.params'

Solution: Setting up pipx with python -m pip install pipx packaging==23.1 (thanks for this !)

@hussein-awala
Copy link
Member Author

Some CI jobs load the scripts files from main (

@hussein-awala (and others) for the future - the way for committers to test such a PR is to make PR from "airflow" repo, not from a fork. Maintainers can push branches directly to apache/airflow repo and PRs from main are build (including building the image) using the scripts coming from the branch that maintainer pushed to the repo.

That's this flow: https://github.com/apache/airflow/blob/main/CI_DIAGRAMS.md#pull-request-flow-from-apacheairflow-repo

And for the record - this whole setup is done in order to achieve security - users from their fork should not be able to modify code that is run in "Github Actions" - because this code can be harmful and get access to GITHUB_TOKEN, write permissions and other sensitive data (secrets).

This is also one of the requirements by the ASF to enable 'run workflows" automatically for forks for external contributors - otherwise we would have to manually approve every single workflow run.

Thank you @potiuk for the clarification, this is very interesting.

I see the CI fails in #34775 although it cherry-pick this commit and source branch is in Apache repository, do you have any idea?

@potiuk
Copy link
Member

potiuk commented Oct 8, 2023

I see the CI fails in #34775 although it cherry-pick this commit and source branch is in Apache repository, do you have any idea?

Yes. We can ignore it. This failure can be ignored. "Pull request target" is run for all PRs but it does "nothing" when PR comes from the "apache" repository based on build info output (but in this case the 'build info" step is failing). As you can see there, all the tests are running and succeeding. Once merged, the build will succeed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:dev-tools changelog:skip Changes that should be skipped from the changelog (CI, tests, etc..)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants