ramitkataria commented on code in PR #47035:
URL: https://github.com/apache/airflow/pull/47035#discussion_r1978421674
##########
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:
I could check for `"Message": "No Airflow role granted in IAM."` but it
doesn't look like this is documented anywhere so if the message ever changes,
this functionality would break. If it was because of some other kind of access
issue, an error message describing the actual issue would get printed either
way during the 2nd attempt. But on the other hand, not checking the message
would create unnecessary info logs when the reason could be something else. So
I'm not sure if this tradeoff is worth it. What do you think?
--
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]
