Taragolis commented on code in PR #38787:
URL: https://github.com/apache/airflow/pull/38787#discussion_r1554540352
##########
airflow/providers/alibaba/cloud/hooks/oss.py:
##########
@@ -338,30 +338,30 @@ def get_credential(self) -> oss2.auth.Auth:
extra_config = self.oss_conn.extra_dejson
auth_type = extra_config.get("auth_type", None)
if not auth_type:
- raise Exception("No auth_type specified in extra_config. ")
+ raise ValueError("No auth_type specified in extra_config. ")
if auth_type != "AK":
- raise Exception(f"Unsupported auth_type: {auth_type}")
+ raise TypeError(f"Unsupported auth_type: {auth_type}")
oss_access_key_id = extra_config.get("access_key_id", None)
oss_access_key_secret = extra_config.get("access_key_secret", None)
if not oss_access_key_id:
- raise Exception(f"No access_key_id is specified for connection:
{self.oss_conn_id}")
+ raise ValueError(f"No access_key_id is specified for connection:
{self.oss_conn_id}")
if not oss_access_key_secret:
- raise Exception(f"No access_key_secret is specified for
connection: {self.oss_conn_id}")
+ raise ValueError(f"No access_key_secret is specified for
connection: {self.oss_conn_id}")
return oss2.Auth(oss_access_key_id, oss_access_key_secret)
def get_default_region(self) -> str:
extra_config = self.oss_conn.extra_dejson
auth_type = extra_config.get("auth_type", None)
if not auth_type:
- raise Exception("No auth_type specified in extra_config. ")
+ raise ValueError("No auth_type specified in extra_config. ")
if auth_type != "AK":
- raise Exception(f"Unsupported auth_type: {auth_type}")
+ raise TypeError(f"Unsupported auth_type: {auth_type}")
Review Comment:
```suggestion
raise ValueError(f"Unsupported auth_type: {auth_type}")
```
##########
airflow/providers/alibaba/cloud/hooks/oss.py:
##########
@@ -338,30 +338,30 @@ def get_credential(self) -> oss2.auth.Auth:
extra_config = self.oss_conn.extra_dejson
auth_type = extra_config.get("auth_type", None)
if not auth_type:
- raise Exception("No auth_type specified in extra_config. ")
+ raise ValueError("No auth_type specified in extra_config. ")
if auth_type != "AK":
- raise Exception(f"Unsupported auth_type: {auth_type}")
+ raise TypeError(f"Unsupported auth_type: {auth_type}")
Review Comment:
TypeError more related to the invalid type as object, e.g. expected sting
but got list and something similar. So better to use ValueError. Maybe also
there is some more better type also exists because ValueError it is also could
be about a lot of things, however it way better that just Exception
```suggestion
raise ValueError(f"Unsupported auth_type: {auth_type}")
```
--
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: commits-unsubscr...@airflow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
