o-nikolas commented on code in PR #47035:
URL: https://github.com/apache/airflow/pull/47035#discussion_r1977934896
##########
providers/amazon/src/airflow/providers/amazon/aws/hooks/mwaa.py:
##########
@@ -56,30 +64,78 @@ def invoke_rest_api(
:param env_name: name of the MWAA environment
:param path: Apache Airflow REST API endpoint path to be called
- :param method: HTTP method used for making Airflow REST API calls
+ :param method: HTTP method used for making Airflow REST API calls:
'GET'|'PUT'|'POST'|'PATCH'|'DELETE'
:param body: Request body for the Apache Airflow REST API call
:param query_params: Query parameters to be included in the Apache
Airflow REST API call
+ :param only_use_web_login: If True, only the web login method is used
without trying boto's
Review Comment:
nit: maybe just `generate_local_token` since a token is being created either
way? It's just who/where it's done?
##########
providers/amazon/src/airflow/providers/amazon/aws/hooks/mwaa.py:
##########
@@ -56,30 +64,78 @@ def invoke_rest_api(
:param env_name: name of the MWAA environment
:param path: Apache Airflow REST API endpoint path to be called
- :param method: HTTP method used for making Airflow REST API calls
+ :param method: HTTP method used for making Airflow REST API calls:
'GET'|'PUT'|'POST'|'PATCH'|'DELETE'
:param body: Request body for the Apache Airflow REST API call
:param query_params: Query parameters to be included in the Apache
Airflow REST API call
+ :param only_use_web_login: If True, only the web login method is used
without trying boto's
+ invoke_rest_api first
"""
- body = body or {}
+ # Filter out keys with None values because Airflow REST API doesn't
accept requests otherwise
+ body = {k: v for k, v in body.items() if v is not None} if body else {}
+ query_params = query_params or {}
api_kwargs = {
"Name": env_name,
"Path": path,
"Method": method,
- # Filter out keys with None values because Airflow REST API
doesn't accept requests otherwise
- "Body": {k: v for k, v in body.items() if v is not None},
- "QueryParameters": query_params if query_params else {},
+ "Body": body,
+ "QueryParameters": query_params,
}
+
+ if only_use_web_login:
+ return self._invoke_rest_api_using_web_login(**api_kwargs)
+
try:
- result = self.conn.invoke_rest_api(**api_kwargs)
+ response = self.conn.invoke_rest_api(**api_kwargs)
# ResponseMetadata is removed because it contains data that is
either very unlikely to be useful
# in XComs and logs, or redundant given the data already included
in the response
- result.pop("ResponseMetadata", None)
- return result
+ response.pop("ResponseMetadata", None)
+ return response
+
except ClientError as e:
- to_log = e.response
- # ResponseMetadata and Error are removed because they contain data
that is either very unlikely to
- # be useful in XComs and logs, or redundant given the data already
included in the response
- to_log.pop("ResponseMetadata", None)
- to_log.pop("Error", None)
- self.log.error(to_log)
- raise e
+ if e.response["Error"]["Code"] == "AccessDeniedException":
+ self.log.info(
+ "Access Denied, possibly due to missing
airflow:InvokeRestApi in IAM policy. "
+ "Trying again using web login..."
+ )
+ return self._invoke_rest_api_using_web_login(**api_kwargs)
Review Comment:
What if this throws?
##########
providers/amazon/src/airflow/providers/amazon/aws/hooks/mwaa.py:
##########
@@ -56,30 +64,78 @@ def invoke_rest_api(
:param env_name: name of the MWAA environment
:param path: Apache Airflow REST API endpoint path to be called
- :param method: HTTP method used for making Airflow REST API calls
+ :param method: HTTP method used for making Airflow REST API calls:
'GET'|'PUT'|'POST'|'PATCH'|'DELETE'
:param body: Request body for the Apache Airflow REST API call
:param query_params: Query parameters to be included in the Apache
Airflow REST API call
+ :param only_use_web_login: If True, only the web login method is used
without trying boto's
+ invoke_rest_api first
"""
- body = body or {}
+ # Filter out keys with None values because Airflow REST API doesn't
accept requests otherwise
+ body = {k: v for k, v in body.items() if v is not None} if body else {}
+ query_params = query_params or {}
api_kwargs = {
"Name": env_name,
"Path": path,
"Method": method,
- # Filter out keys with None values because Airflow REST API
doesn't accept requests otherwise
- "Body": {k: v for k, v in body.items() if v is not None},
- "QueryParameters": query_params if query_params else {},
+ "Body": body,
+ "QueryParameters": query_params,
}
+
+ if only_use_web_login:
+ return self._invoke_rest_api_using_web_login(**api_kwargs)
+
try:
- result = self.conn.invoke_rest_api(**api_kwargs)
+ response = self.conn.invoke_rest_api(**api_kwargs)
# ResponseMetadata is removed because it contains data that is
either very unlikely to be useful
# in XComs and logs, or redundant given the data already included
in the response
- result.pop("ResponseMetadata", None)
- return result
+ response.pop("ResponseMetadata", None)
+ return response
+
except ClientError as e:
- to_log = e.response
- # ResponseMetadata and Error are removed because they contain data
that is either very unlikely to
- # be useful in XComs and logs, or redundant given the data already
included in the response
- to_log.pop("ResponseMetadata", None)
- to_log.pop("Error", None)
- self.log.error(to_log)
- raise e
+ if e.response["Error"]["Code"] == "AccessDeniedException":
+ self.log.info(
+ "Access Denied, possibly due to missing
airflow:InvokeRestApi in IAM policy. "
+ "Trying again using web login..."
Review Comment:
Is there no way to be certain by inspecting the error `message`?
--
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]
