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 2020/10/10 13:21:25 UTC

[GitHub] [airflow] mcepok commented on a change in pull request #11219: [AIRFLOW-6585] Fixed Timestamp bug in RefreshKubeConfigLoader

mcepok commented on a change in pull request #11219:
URL: https://github.com/apache/airflow/pull/11219#discussion_r502790183



##########
File path: airflow/kubernetes/refresh_config.py
##########
@@ -24,15 +24,22 @@
 import logging
 import os
 import time
-from datetime import datetime
 from typing import Optional
 
+import pendulum
 import yaml
 from kubernetes.client import Configuration
 from kubernetes.config.exec_provider import ExecProvider
 from kubernetes.config.kube_config import KUBE_CONFIG_DEFAULT_LOCATION, KubeConfigLoader
 
 
+def _parse_timestamp(ts_str: str) -> int:
+    parsed_dt = pendulum.parse(ts_str)
+    if isinstance(parsed_dt, pendulum.DateTime):

Review comment:
       The problem is, that pendulum.parse type annotations says it return `typing.Union[Date, Time, DateTime, Duration]`, however not all of these implement `timetuple()`.
   One of the CI tests (I guess it was mypy) yelled at me for not covering the other possible return types. I was not able to find an input string which didn't result in either a parser error or in a return type DateTime so it doesn't really matter I guess.




----------------------------------------------------------------
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.

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