Eason09053360 commented on code in PR #70991:
URL: https://github.com/apache/airflow/pull/70991#discussion_r3977932156


##########
airflow-ctl/src/airflowctl/api/client.py:
##########


Review Comment:
   I think now that the handler is gone, this `# type: ignore[arg-type]` is the 
only thing acknowledging that `api_token` is `str | None`, and nothing guards a 
`None` token before it reaches keyring. What happens then is backend-specific: 
`keyrings.alt` raises a `TypeError` about *bytes*, macOS raises 
`PasswordSetError`, Windows silently stores the literal string `"None"`. Since 
the point of the PR is that `save()` should never be reached without a token, 
let's make that explicit and let mypy check the argument again:
   
   ```suggestion
                   if self.api_token is None:
                       raise AirflowCtlCredentialNotFoundException("No API 
token found. Please login first.")
                   keyring.set_password(
                       "airflowctl",
                       self.token_key_for_environment(self.api_environment),
                       self.api_token,
                   )
   ```
   
   



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