Skip to content

Comments

Optimise getting changelogs in airflow-github#36548

Merged
amoghrajesh merged 2 commits intoapache:mainfrom
amoghrajesh:airflowGitScriptFix
Jan 5, 2024
Merged

Optimise getting changelogs in airflow-github#36548
amoghrajesh merged 2 commits intoapache:mainfrom
amoghrajesh:airflowGitScriptFix

Conversation

@amoghrajesh
Copy link
Contributor

Fix the logic for getting changelog in minor versions so that we get only the changelog that is not present in the RELEASE_NOTES.rst instead of getting everything which is a heavy operation.

How was this tested?

  1. Got the result as-is: ./airflow-github changelog origin/v1-8-stable origin/v1-8-test > without-changes
  2. Made the changes and then got the results: ./airflow-github changelog origin/v1-8-stable origin/v1-8-test > with-changes
  3. Manually verified the content to be same
  4. Checked using diff tool: git diff --no-index with-changes without-changes

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

@ephraimbuddy
Copy link
Contributor

Can you test by running it between v2-7-stable and v2-8-stable. Problem was between minors

@amoghrajesh
Copy link
Contributor Author

Sure @ephraimbuddy, let me check that out and I will post the results here

@amoghrajesh
Copy link
Contributor Author

@ephraimbuddy there was a bug in the code and I managed to fix it now after some debugging. The issue was that the non core commits were getting included due to faulty else condition.

Managed to test between origin/v2-7-stable and origin/v2-8-stable.

Results:

  1. File sizes:
root@8738c81c51bf:/opt/airflow/dev# ls -lh
total 508K
-rw-r--r--  1 root root  29K Jan  3 20:15 2-7-to-2-8-newchanges
-rw-r--r--  1 root root  29K Jan  3 17:20 2-7-to-2-8-original
  1. Diff:
root@8738c81c51bf:/opt/airflow/dev# git diff --no-index 2-7-to-2-8-original 2-7-to-2-8-newchanges
diff --git a/2-7-to-2-8-original b/2-7-to-2-8-newchanges
index 78fda81..1862824 100644
--- a/2-7-to-2-8-original
+++ b/2-7-to-2-8-newchanges
@@ -1,4 +1,4 @@
-len of commits 1700
+Number of commits 1700
 Uncategorized
 """""""""""""
 - Update RELEASE_NOTES.rst

image

The diff is due to different log message, not any actual diff.

@amoghrajesh
Copy link
Contributor Author

The new approach also saves a hell lot of time actually :)
Realised because this command is super time consuming!

@potiuk
Copy link
Member

potiuk commented Jan 3, 2024

@ephraimbuddy - WDYT?

@amoghrajesh
Copy link
Contributor Author

Gentle bump on this one @ephraimbuddy

@amoghrajesh
Copy link
Contributor Author

Thank you. Sending this one in, since it is a simple fix.

Thanks @potiuk @ephraimbuddy @Lee-W for the reviews

@amoghrajesh amoghrajesh merged commit 142c737 into apache:main Jan 5, 2024
@potiuk potiuk added this to the Airflow 2.8.1 milestone Jan 13, 2024
@potiuk potiuk added the changelog:skip Changes that should be skipped from the changelog (CI, tests, etc..) label Jan 13, 2024
potiuk pushed a commit that referenced this pull request Jan 13, 2024
ephraimbuddy pushed a commit that referenced this pull request Jan 15, 2024
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.

4 participants