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

Migrating issue generation to breeze release-management #38062

Merged
merged 12 commits into from
Mar 16, 2024

Conversation

amoghrajesh
Copy link
Contributor

@amoghrajesh amoghrajesh commented Mar 12, 2024

Offline chat with @eladkal revealed that there is an inconsistency between how issues are generated for providers, helm-charts, and airflow. Initially I had fixed #37891 and realised that a bigger effort is needed to move everything to breeze. This PR moves issue generation from prepare_release_issue.py to breeze release-management and introduces two new commands.

The usage looks like this:

breeze release-management generate-issue-content-core --previous-release 1.9.0 --current-release 1.10.0 --verbose
breeze release-management generate-issue-content-helm-chart --previous-release 1.6.0 --current-release 1.8.0 --verbose

Tested for helm chart as well as airflow core.

Example output:

root@d4e667edbe5e:~/clone/airflow# ./dev/prepare_release_issue.py generate-issue-content --previous-release 1.9.0 --current-release ${VERSION}
Retrieving PR#3701: https://github.com/apache/airflow/pull/3701
Retrieving PR#3720: https://github.com/apache/airflow/pull/3720
Retrieving PR#3708: https://github.com/apache/airflow/pull/3708
Retrieving PR#3700: https://github.com/apache/airflow/pull/3700
Retrieving PR#3703: https://github.com/apache/airflow/pull/3703
Retrieving PR#3660: https://github.com/apache/airflow/pull/3660
Retrieving PR#24: https://github.com/apache/airflow/pull/24
Retrieving 7 PRs  ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 100% 0:00:00
We are kindly requesting that contributors to [Apache Airflow RC 1.10.0](https://pypi.org/project/apache-airflow/1.10.0/) help test the RC.

Please let us know by commenting if the issue is addressed in the latest RC.

- [ ] [Fix run_unit_tests.sh (#24)](https://github.com/apache/airflow/pull/24): @afarbos
- [ ] [[AIRFLOW-2817] Force explicit choice on GPL dependency (#3660)](https://github.com/apache/airflow/pull/3660): @bolkedebruin
- [ ] [[AIRFLOW-2140] Don't require kubernetes for the SparkSubmit hook (#3700)](https://github.com/apache/airflow/pull/3700): @ashb
- [ ] [[AIRFLOW-2856] Pass in SLUGIFY_USES_TEXT_UNIDECODE=yes ENV to docker run (#3701)](https://github.com/apache/airflow/pull/3701): @andscoop
- [ ] [[AIRFLOW-2857] Fix broken RTD env  (#3703)](https://github.com/apache/airflow/pull/3703): @tedmiston
- [ ] [[AIRFLOW-2859] Implement own UtcDateTime (#3708)](https://github.com/apache/airflow/pull/3708): @bolkedebruin
- [ ] [[AIRFLOW-2870] Use abstract TaskInstance for migration (#3720)](https://github.com/apache/airflow/pull/3720): @bolkedebruin


Thanks to all who contributed to the release (probably not a complete list!):
@afarbos @andscoop @tedmiston @ashb @bolkedebruin

(new-env) ➜  airflow git:(issueContentToBreeze) ✗ breeze release-management generate-issue-content-core --previous-release 1.9.0 --current-release 1.10.0 --verbose
/Users/adesai/.local/pipx/venvs/apache-airflow-breeze/lib/python3.9/site-packages/urllib3/__init__.py:34: NotOpenSSLWarning: urllib3 v2.0 only supports OpenSSL 1.1.1+, currently the 'ssl' module is compiled with 'LibreSSL 2.8.3'. See: https://github.com/urllib3/urllib3/issues/3020
  warnings.warn(
/Users/adesai/Documents/OSS/airflow/dev/breeze/src/airflow_breeze/commands /Users/adesai/Documents/OSS/airflow
Command to run: 'git log --pretty=format:%H %h %cd %s --date=short 1.9.0...1.10.0 -- .'
Retrieving PR#3701: https://github.com/apache/airflow/pull/3701
Retrieving PR#3720: https://github.com/apache/airflow/pull/3720
Retrieving PR#3708: https://github.com/apache/airflow/pull/3708
Retrieving PR#3700: https://github.com/apache/airflow/pull/3700
Retrieving PR#3703: https://github.com/apache/airflow/pull/3703
Retrieving PR#3660: https://github.com/apache/airflow/pull/3660
Retrieving PR#24: https://github.com/apache/airflow/pull/24
Retrieving 7 PRs  ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 100% 0:00:00
We are kindly requesting that contributors to [Apache Airflow RC 1.10.0](https://pypi.org/project/apache-airflow/1.10.0/) help test the RC.

Please let us know by commenting if the issue is addressed in the latest RC.

- [ ] [Fix run_unit_tests.sh (#24)](https://github.com/apache/airflow/pull/24): @afarbos
- [ ] [[AIRFLOW-2817] Force explicit choice on GPL dependency (#3660)](https://github.com/apache/airflow/pull/3660): @bolkedebruin
- [ ] [[AIRFLOW-2140] Don't require kubernetes for the SparkSubmit hook (#3700)](https://github.com/apache/airflow/pull/3700): @ashb
- [ ] [[AIRFLOW-2856] Pass in SLUGIFY_USES_TEXT_UNIDECODE=yes ENV to docker run (#3701)](https://github.com/apache/airflow/pull/3701): @andscoop
- [ ] [[AIRFLOW-2857] Fix broken RTD env  (#3703)](https://github.com/apache/airflow/pull/3703): @tedmiston
- [ ] [[AIRFLOW-2859] Implement own UtcDateTime (#3708)](https://github.com/apache/airflow/pull/3708): @bolkedebruin
- [ ] [[AIRFLOW-2870] Use abstract TaskInstance for migration (#3720)](https://github.com/apache/airflow/pull/3720): @bolkedebruin


Thanks to all who contributed to the release (probably not a complete list!):
@bolkedebruin @afarbos @ashb @tedmiston @andscoop


^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in newsfragments.

Copy link
Member

@potiuk potiuk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NAAAJS.. Few comments:

  • we should remove the dev/* script in the same PR

We should run the command in CI together right after Airflow Release And Helm Release commands - this way we will be sure that the command works (at least it will not break in the CI).

We already do that in case of providers:

      - name: "Test providers issue generation automatically"
        run: >
          breeze release-management generate-issue-content-providers
          --only-available-in-dist --disable-progress

In the prepare-test-provider-packages-wheel job.

For Helm it should be added in tests-helm-release jon
For Airlfow it should be added prepare-test-provider-packages-wheel as this is where we also prepare airflow package

      - name: "Prepare airflow package: wheel"
        run: breeze release-management prepare-airflow-package --version-suffix-for-pypi dev0

This is the main benefit of having the commands nicely packaged as breeze commands - because we can run them in CI and we can be reasonably sure they continue working.

Also I guess there will be a follow up PR where we - possibly - do some refactor and extract the common parts of generate_issue_content_* methods ? It looks like there are quite a lot of duplicated lines of code that could by DRY-ed (but I believe it shoudl be a follow-up PR once we get one or two rounds of releases and this will be proven to work - we might avoid some unnecessary coupling issues this way.

@potiuk
Copy link
Member

potiuk commented Mar 13, 2024

(also having it running in CI is ultimate proof that it works - and there is no need for the reviewer to run it locally - they will simply be able to see it in the CI :)

@amoghrajesh
Copy link
Contributor Author

amoghrajesh commented Mar 13, 2024

We already do that in case of providers:

Understood. Will make the changes for this

@amoghrajesh
Copy link
Contributor Author

we should remove the dev/* script in the same PR

Yeah I wanted to, but wasn't sure whether to keep this in case it doesn't work.

@amoghrajesh
Copy link
Contributor Author

Also I guess there will be a follow up PR where we - possibly - do some refactor and extract the common parts of generate_issue_content_* methods ? It looks like there are quite a lot of duplicated lines of code that could by DRY-ed (but I believe it shoudl be a follow-up PR once we get one or two rounds of releases and this will be proven to work - we might avoid some unnecessary coupling issues this way.

Of course, I will be refactoring in a follow up. I know its messy :)

@amoghrajesh amoghrajesh requested a review from potiuk March 13, 2024 15:05
@amoghrajesh
Copy link
Contributor Author

@potiuk @eladkal I added it to CI as well, wasn't so sure what version to use, so I used some random versions. WDYT?

@potiuk
Copy link
Member

potiuk commented Mar 13, 2024

@potiuk @eladkal I added it to CI as well, wasn't so sure what version to use, so I used some random versions. WDYT?

It will get longer and longer over time. Ideally, we should do the same what the release manager does - i.e use latest version. It's rather easy for airflow - you could potentiallly add support for latest value and find latest version from Pypi (already used in Breeze):

    response = requests.get("https://pypi.org/pypi/apache-airflow/json")
    response.raise_for_status()
    latest_released_version = response.json()["info"]["version"]

For Helm chart it shoud also be somewhat easy to get latest version ? I leave that exercise to you

@amoghrajesh
Copy link
Contributor Author

Ok understood, I can take it up in a follow up once this is stabilised. Your call, let me know

@potiuk
Copy link
Member

potiuk commented Mar 13, 2024

Sure. Can be done separately :)

@potiuk
Copy link
Member

potiuk commented Mar 13, 2024

Those issues generated look a bit short though :) (Helm one is empty :( )

image

@amoghrajesh
Copy link
Contributor Author

Helm one has content right? You mean airflow one?

@potiuk
Copy link
Member

potiuk commented Mar 13, 2024

Compare it with the provider ones (And that's only part of it):

image

@potiuk
Copy link
Member

potiuk commented Mar 13, 2024

Helm is currently this:

image

@amoghrajesh
Copy link
Contributor Author

@potiuk requesting small guidance here. I am hardcoding the versions as helm-chart/0.0.0 and helm-chart/0.0.0dev0 which is being tagged by previous stages in the workflow, not sure why it fails. Can i get some help here?

`

2024-03-14T11:35:48.9417362Z ##[group]Run breeze release-management generate-issue-content-helm-chart --previous-release helm-chart/0.0.0 --current-release helm-chart/0.0.0dev0
2024-03-14T11:35:48.9418626Z �[36;1mbreeze release-management generate-issue-content-helm-chart --previous-release helm-chart/0.0.0 --current-release helm-chart/0.0.0dev0�[0m
2024-03-14T11:35:48.9432419Z shell: /usr/bin/bash -e {0}
2024-03-14T11:35:48.9432738Z env:
2024-03-14T11:35:48.9432982Z RUNS_ON: ["self-hosted", "Linux", "X64"]
2024-03-14T11:35:48.9433345Z PYTHON_MAJOR_MINOR_VERSION: 3.8
2024-03-14T11:35:48.9433809Z pythonLocation: /home/runner/actions-runner/_work/_tool/Python/3.9.18/x64
2024-03-14T11:35:48.9434470Z PKG_CONFIG_PATH: /home/runner/actions-runner/_work/_tool/Python/3.9.18/x64/lib/pkgconfig
2024-03-14T11:35:48.9435126Z Python_ROOT_DIR: /home/runner/actions-runner/_work/_tool/Python/3.9.18/x64
2024-03-14T11:35:48.9435720Z Python2_ROOT_DIR: /home/runner/actions-runner/_work/_tool/Python/3.9.18/x64
2024-03-14T11:35:48.9436311Z Python3_ROOT_DIR: /home/runner/actions-runner/_work/_tool/Python/3.9.18/x64
2024-03-14T11:35:48.9436946Z LD_LIBRARY_PATH: /home/runner/actions-runner/_work/_tool/Python/3.9.18/x64/lib
2024-03-14T11:35:48.9437419Z ##[endgroup]
2024-03-14T11:35:49.2385670Z fatal: bad revision 'helm-chart/0.0.0...helm-chart/0.0.0dev0'
2024-03-14T11:35:49.2387917Z Traceback (most recent call last):
2024-03-14T11:35:49.2393691Z File "/home/runner/.local/bin/breeze", line 8, in
2024-03-14T11:35:49.2395320Z /home/runner/actions-runner/_work/airflow/airflow/dev/breeze/src/airflow_breeze/commands /home/runner/actions-runner/_work/airflow/airflow
2024-03-14T11:35:49.2396218Z sys.exit(main())
2024-03-14T11:35:49.2397171Z File "/home/runner/.local/pipx/venvs/apache-airflow-breeze/lib/python3.9/site-packages/click/core.py", line 1157, in call
2024-03-14T11:35:49.2397913Z return self.main(*args, **kwargs)
2024-03-14T11:35:49.2398768Z File "/home/runner/.local/pipx/venvs/apache-airflow-breeze/lib/python3.9/site-packages/rich_click/rich_command.py", line 126, in main
2024-03-14T11:35:49.2399614Z rv = self.invoke(ctx)
2024-03-14T11:35:49.2400360Z File "/home/runner/.local/pipx/venvs/apache-airflow-breeze/lib/python3.9/site-packages/click/core.py", line 1688, in invoke
2024-03-14T11:35:49.2401123Z return _process_result(sub_ctx.command.invoke(sub_ctx))
2024-03-14T11:35:49.2402078Z File "/home/runner/.local/pipx/venvs/apache-airflow-breeze/lib/python3.9/site-packages/click/core.py", line 1688, in invoke
2024-03-14T11:35:49.2402928Z return _process_result(sub_ctx.command.invoke(sub_ctx))
2024-03-14T11:35:49.2403891Z File "/home/runner/.local/pipx/venvs/apache-airflow-breeze/lib/python3.9/site-packages/click/core.py", line 1434, in invoke
2024-03-14T11:35:49.2404631Z return ctx.invoke(self.callback, **ctx.params)
2024-03-14T11:35:49.2405471Z File "/home/runner/.local/pipx/venvs/apache-airflow-breeze/lib/python3.9/site-packages/click/core.py", line 783, in invoke
2024-03-14T11:35:49.2406270Z return __callback(*args, **kwargs)
2024-03-14T11:35:49.2407332Z File "/home/runner/actions-runner/_work/airflow/airflow/dev/breeze/src/airflow_breeze/commands/release_management_commands.py", line 2265, in generate_issue_content_helm_chart
2024-03-14T11:35:49.2408624Z changes = get_changes(verbose, previous_release, current_release, is_helm_chart)
2024-03-14T11:35:49.2409756Z File "/home/runner/actions-runner/_work/airflow/airflow/dev/breeze/src/airflow_breeze/commands/release_management_commands.py", line 2148, in get_changes
2024-03-14T11:35:49.2410624Z change_strings = subprocess.check_output(
2024-03-14T11:35:49.2411421Z File "/home/runner/actions-runner/_work/_tool/Python/3.9.18/x64/lib/python3.9/subprocess.py", line 424, in check_output
2024-03-14T11:35:49.2412197Z return run(*popenargs, stdout=PIPE, timeout=timeout, check=True,
2024-03-14T11:35:49.2413021Z File "/home/runner/actions-runner/_work/_tool/Python/3.9.18/x64/lib/python3.9/subprocess.py", line 528, in run
2024-03-14T11:35:49.2413962Z raise CalledProcessError(retcode, process.args,
2024-03-14T11:35:49.2415056Z subprocess.CalledProcessError: Command '['git', 'log', '--pretty=format:%H %h %cd %s', '--date=short', 'helm-chart/0.0.0...helm-chart/0.0.0dev0', '--', 'chart/']' returned non-zero exit status 128.
2024-03-14T11:35:49.2871904Z ##[error]Process completed wi

`

@potiuk
Copy link
Member

potiuk commented Mar 14, 2024

I don't think tagging is done? I could not find it actually ? Where is it tagged? I believe the previous steps only use existing tags but never tag it ? Or maybe I missed it ?

@amoghrajesh
Copy link
Contributor Author

I don't think tagging is done? I could not find it actually ? Where is it tagged? I believe the previous steps only use existing tags but never tag it ? Or maybe I missed it ?

Maybe I am slightly mistaken here but I was checking the process logs.
We have this happening in the step "Helm release tarball"

Run breeze release-management prepare-helm-chart-tarball --ignore-version-check --override-tag --skip-tag-signing --version 0.0.0 --version-suffix dev0
Airflow version in values.yaml: 2.8.3
Default Airflow Tag in values.yaml: 2.8.3
Airflow version in Chart.yaml: 2.8.3
Version in chart.yaml (1.[14](https://github.com/apache/airflow/actions/runs/8278547604/job/22651378456?pr=38062#step:9:15).0-dev) does not match the version passed as parameter (0.0.0). Updating
Airflow version in chart.yaml matches the airflow version in values.yaml: (2.8.3)

Versions of the chart has been updated

Ignoring the version check. The tarball will be created but it should not be published
Tagging the chart with helm-chart/0.0.0dev0
Creating tarball for Helm Chart helm-chart/0.0.0dev0
Tarball created in /home/runner/actions-runner/_work/airflow/airflow/dist/airflow-chart-0.0.0-source.tar.gz

And in this breeze command the following happens

    if not skip_tagging:
        get_console().print(f"[info]Tagging the chart with {tag_with_suffix}[/]")
        tag_command = [
            "git",
            "tag",
            tag_with_suffix,
            "-m",
            f"Apache Airflow Helm Chart {version}{version_suffix}",
        ]
        if override_tag:
            tag_command.append("--force")
        if not skip_tag_signing:
            tag_command.append("--sign")
        result = run_command(tag_command, check=False)
        if result.returncode != 0:
            get_console().print(f"[error]Error tagging the chart with {tag_with_suffix}.\n")
            get_console().print(
                "[warning]If you are sure the tag is set correctly, you can add --skip-tagging"
                " flag to the command[/]"
            )
            sys.exit(result.returncode)
    else:
        get_console().print(f"[warning]Skipping tagging the chart with {tag_with_suffix}[/]")
    get_console().print(f"[info]Creating tarball for Helm Chart {tag_with_suffix}[/]")

So I think it has a tag now. I will check out what the right one is

@amoghrajesh
Copy link
Contributor Author

@potiuk I ran a git tag in CI environment and it does have one tag: helm-chart/0.0.0dev0
I have added it for helm chart now, I will unblock it in a followup where i allow it to get the realistic versions and work on that instead.

@amoghrajesh
Copy link
Contributor Author

The job has passed now: https://github.com/apache/airflow/actions/runs/8292193216/job/22693362907?pr=38062

If all is ok, we can send this one in and I will pick up the follow ups asap.

@amoghrajesh
Copy link
Contributor Author

@potiuk can you take a round at this once you have some time? Just rebased onto the new HEAD of main :)

@potiuk potiuk merged commit 8660eef into apache:main Mar 16, 2024
90 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants