eladkal commented on code in PR #37538:
URL: https://github.com/apache/airflow/pull/37538#discussion_r1494366875
##########
airflow/providers/google/cloud/operators/bigquery_dts.py:
##########
@@ -359,7 +369,18 @@ def _wait_for_transfer_to_be_done(self, run_id: str,
transfer_config_id: str, in
if interval <= 0:
raise ValueError("Interval must be > 0")
+ idx = 0
while True:
+ current_tick_div, current_tick_mod = divmod(idx * interval,
self.token_refresh_interval_seconds)
+ next_tick_div, next_tick_mod = divmod((idx + 1) * interval,
self.token_refresh_interval_seconds)
+ if (current_tick_div < next_tick_div and 0 < next_tick_mod) or (
+ current_tick_mod == 0 and 1 <= current_tick_div
+ ):
+ _ = self.hook.refresh_credentials()
+ self.log.info(
+ f"Credentials were refreshed on tick: idx={idx},
idx*interval={idx * interval} sec"
+ )
Review Comment:
Other providers also requires refresh logic but seems like they have simpler
implementation. For example:
https://github.com/apache/airflow/blob/0b680c94922e3f7ca1f3ada8328e315bbae37dc8/airflow/providers/databricks/hooks/databricks_base.py#L441-L453
Can you explain why we need complex logic here?
Also please extract the check validation (time handling) to it's own private
function - it should be covered by unit tests
--
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]
