Re: [PR] Fix(df): Remove obsolete version check in doctest checker & fix rate limit hits [beam]

[via GitHub]

gemini-code-assist[bot] commented on code in PR #35727:
URL: https://github.com/apache/beam/pull/35727#discussion_r2240368202


##########
.github/actions/setup-python/action.yml:
##########
@@ -0,0 +1,38 @@
+name: 'Setup Python Environment'
+description: 'Installs Python and caches dependencies'
+inputs:
+  python-version:
+    required: true
+    description: 'Install Python version'
+  python-cache:
+    required: false
+    description: 'Whether to enable Python pip caching'
+    default: true
+  tox-cache:
+    required: false
+    description: 'Whether to enable tox environment caching'
+    default: true
+runs:
+  using: "composite"
+  steps:
+    - name: Install Python
+      uses: actions/setup-python@v5
+      with:
+        python-version: ${{ inputs.python-version }}
+        cache: ${{ inputs.python-cache && 'pip' || 'none' }}
+        cache-dependency-path: |
+          sdks/python/setup.py
+          sdks/python/tox.ini
+
+    - name: Cache tox environments
+      if: ${{ inputs.tox-cache == 'true' }}

Review Comment:
   ![high](https://www.gstatic.com/codereviewagent/high-priority.svg)
   
   The `if` condition for caching tox environments is incorrect. The 
`tox-cache` input is a boolean, but it's being compared to the string `'true'`. 
In GitHub Actions expressions, a boolean `true` is not equal to the string 
`'true'`, so this condition will always evaluate to false, and the caching step 
will never run. To fix this, you should directly use the boolean value of the 
input in the condition.
   
   ```
         if: ${{ inputs.tox-cache }}
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscr...@beam.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org