fix(composer): fix version check logic for 'airflow_db_cleanup.py'

[eapl-gemugami]

Description

Fixes: #13288
Fixes Internal: b/408307862

If required, watch the FL in Internal.

• Add a unit test to validate the version comparison
• Add a function to parse the version in a string like 2.9.2+composer
• Convert the version from a list into a tuple, as requested by @gemini-code-assist
• Add Python 3.13 to ignore_versions in noxfile_config.py

Checklist

• I have followed Sample Guidelines from AUTHORING_GUIDE.MD
• Tests pass: nox -s py-3.12 (see Test Environment Setup)
• Lint pass: nox -s lint (see Test Environment Setup)
• Please merge this PR for me once it is approved

[gemini-code-assist]

Hello @eapl-gemugami, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

Summary of Changes

This pull request addresses an issue with the version check logic in the airflow_db_cleanup.py script. The goal is to ensure that the script correctly determines the Airflow version, especially when the version string includes a '+composer' suffix. The changes involve modifying the version comparison logic to handle the suffix and converting the version string into a list of integers for accurate comparisons. Additionally, a test case has been added to airflow_db_cleanup_test.py to validate the version comparison logic.

Highlights

• Version Check Logic: The version check logic in airflow_db_cleanup.py has been updated to correctly parse and compare Airflow versions, including those with a '+composer' suffix.
• Version Conversion: The Airflow version string is now converted into a list of integers to facilitate accurate version comparisons.
• Test Case: A new test case has been added to airflow_db_cleanup_test.py to validate the version comparison logic.
• Python 3.12: Temporarily disabled Python 3.12 testing in noxfile_config.py.

Changelog

Click here to see the changelog

• composer/workflows/airflow_db_cleanup.py
  • Added a note indicating that the sample is intended to work with Airflow 1 and 2 (line 15).
  • Modified the code to handle the '+composer' suffix in the Airflow version string (lines 97-103).
  • Converted the Airflow version string into a list of integers for comparison (line 103).
  • Added a TODO comment to confirm the logic for TaskInstance age check (line 133).
  • Updated version checks for XCom, TaskReschedule, RenderedTaskInstanceFields and BaseJob to use integer list comparison (lines 150, 180, 209, 244).
• composer/workflows/airflow_db_cleanup_test.py
  • Added a function convert_version_str_to_list to convert the version string to a list of integers (lines 19-28).
  • Added a test case test_version_comparison to validate the version comparison logic (lines 31-41).
• composer/workflows/noxfile_config.py
  • Commented out Python 3.12 from the test matrix (line 40).
• composer/workflows/requirements.txt
  • Commented out apache-airflow 2.10.5 requirement (line 8).

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in issue comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

---

A suffix, '+composer',
Hides the version number's core,
Split it, then compare,
Lest bugs forever snare,
Tests ensure versions are sure.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

The pull request aims to fix the version check logic in the airflow_db_cleanup.py script, ensuring compatibility with different Airflow versions, including those with non-numerical characters in the version string. The changes involve updating the version comparison logic and adding a unit test to validate the fix. Overall, the changes look good, but there are a few areas that could be improved for clarity and maintainability.

Summary of Findings

• Clarity of Version Comparison Logic: The version comparison logic could be made more readable by extracting it into a separate function with a descriptive name. This would improve the overall clarity of the code and make it easier to understand the intent.
• TODO Comment Resolution: The TODO comment regarding the TaskInstance age_check_column should be addressed to either confirm the logic or remove the comment. Leaving it unresolved can lead to confusion in the future.
• Test Coverage: The test coverage could be improved by adding more test cases to cover different scenarios, such as versions with release candidates (e.g., '2.9.3rc1') and edge cases. This would ensure that the version comparison logic is robust and reliable.

Merge Readiness

The pull request is almost ready for merging. However, the TODO comment should be addressed, and the version comparison logic could be made more readable. I am unable to approve this pull request, and recommend that others review and approve this code before merging. Addressing the comments would improve the overall quality and maintainability of the code.

[eapl-gemugami]

/gemini review

[gemini-code-assist]

Code Review

The pull request addresses an issue with the version check logic in the airflow_db_cleanup.py script. The changes include a new function to parse Airflow versions, updates to version comparisons, and a unit test for the version parsing logic. Overall, the changes look good and improve the reliability of the script.

Summary of Findings

• TODO comment in parse_airflow_version: The parse_airflow_version function contains a TODO comment indicating that it needs to be updated to handle non-numerical characters in the version string. This should be addressed to ensure the function works correctly with all possible Airflow versions.
• Inconsistent commenting style: There are some inconsistencies in the commenting style, such as using both '#' and '##' for section headers. Standardizing the commenting style would improve readability.
• Version comparison: The version comparison logic has been updated to use tuples instead of lists. This is a good change as tuples are immutable and more suitable for version comparisons.

Merge Readiness

The pull request is almost ready for merging. The changes address the identified issue and include a unit test. However, the TODO comment in parse_airflow_version should be addressed before merging. I am unable to directly approve this pull request, and recommend that others review and approve this code before merging.

[eapl-gemugami]

Reviewers: Please let me know whether pep-0008/#inline-comments applies for requirements.txt as well.

I couldn't find a Style Guide for this file, rather than https://pip.pypa.io/en/stable/reference/requirements-file-format/