Nataneljpwd commented on PR #64504:
URL: https://github.com/apache/airflow/pull/64504#issuecomment-4169947993

   > @Nataneljpwd This PR has a few issues that need to be addressed before it 
can be reviewed — please see our [Pull Request quality 
criteria](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#pull-request-quality-criteria).
   > 
   > **Issues found:**
   > - :warning: **Code Quality**: In `kubernetes_executor.py`, `headers = 
e.headers` is assigned and then `headers.get("Retry-After", "0")` is called 
unconditionally. `ApiException.headers` can be `None`, which would raise 
`AttributeError` at runtime. The code should guard with `if headers:` or use 
`(headers or {}).get(...)`.
   > - :warning: **Code Quality**: `int(headers.get("Retry-After", "0"))` will 
raise `ValueError` if the header value is not a valid integer string (e.g. a 
date-based Retry-After as allowed by RFC 7231). The conversion should be 
wrapped in a try/except or validated before casting.
   > 
   > **What to do next:**
   > - The comment informs you what you need to do.
   > - Fix each issue, then mark the PR as "Ready for review" in the GitHub UI 
- but only after making sure that all the issues are fixed.
   > - There is no rush — take your time and work at your own pace. We 
appreciate your contribution and are happy to wait for updates.
   > - Maintainers will then proceed with a normal review.
   > 
   > There is no rush — take your time and work at your own pace. We appreciate 
your contribution and are happy to wait for updates. If you have questions, 
feel free to ask on the [Airflow Slack](https://s.apache.org/airflow-slack).
   
   Fixed the headers issue, yet about the retry after header, the k8s api 
returns it in seconds, and so the RFC here is not relevant
   
   https://kubernetes.io/docs/reference/using-api/api-concepts/
   
   (Bottom of the page)
   
   Though if you want, I can add a check for a datetime based header


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to