You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2021/10/11 12:27:17 UTC

[GitHub] [airflow] eladkal commented on a change in pull request #18883: Upgrade the graph api to latest veersion

eladkal commented on a change in pull request #18883:
URL: https://github.com/apache/airflow/pull/18883#discussion_r726071027



##########
File path: airflow/providers/facebook/ads/hooks/ads.py
##########
@@ -66,7 +66,7 @@ class FacebookAdsReportingHook(BaseHook):
     def __init__(
         self,
         facebook_conn_id: str = default_conn_name,
-        api_version: str = "v6.0",
+        api_version: str = "v12.0",

Review comment:
       I think it would be better not to set our own default. We can import the default value from the library itself
   https://github.com/facebook/facebook-python-business-sdk/blob/fbc3b6a5c2d54bd4184163b290dd614b668e4ee0/facebook_business/apiconfig.py#L22
   
   or maybe better just not have a default value for `api_version` at all.
   The upstream lib made this parameter optional:
   https://github.com/facebook/facebook-python-business-sdk/blob/fbc3b6a5c2d54bd4184163b290dd614b668e4ee0/facebook_business/api.py#L181
   That way it will use the default value of the upstream lib if the user didn't provide a value.




-- 
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-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org