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/09/28 03:15:43 UTC

[GitHub] [airflow] subkanthi opened a new pull request #18563: Test kubernetes refresh config

subkanthi opened a new pull request #18563:
URL: https://github.com/apache/airflow/pull/18563


   <!--
   Thank you for contributing! Please make sure that your code changes
   are covered with tests. And in case of new features or big changes
   remember to adjust the documentation.
   
   Feel free to ping committers for the review!
   
   In case of existing issue, reference it using one of the following:
   
   closes: #ISSUE
   related: #ISSUE
   
   How to write a good git commit message:
   http://chris.beams.io/posts/git-commit/
   -->
   
   ---
   **^ Add meaningful description above**
   
   Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst#pull-request-guidelines)** for more information.
   In case of fundamental code change, Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvements+Proposals)) is needed.
   In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
   In case of backwards incompatible changes please leave a note in [UPDATING.md](https://github.com/apache/airflow/blob/main/UPDATING.md).
   


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



[GitHub] [airflow] subkanthi commented on a change in pull request #18563: Test kubernetes refresh config

Posted by GitBox <gi...@apache.org>.
subkanthi commented on a change in pull request #18563:
URL: https://github.com/apache/airflow/pull/18563#discussion_r717662037



##########
File path: tests/kubernetes/test_refresh_config.py
##########
@@ -35,3 +43,64 @@ def test_parse_timestamp_should_convert_regular_timezone_to_unix_timestamp(self)
     def test_parse_timestamp_should_throw_exception(self):
         with pytest.raises(ParserError):
             _parse_timestamp("foobar")
+
+    def test_get_kube_config_loader_for_yaml_file(self):
+        refresh_kube_config_loader = _get_kube_config_loader_for_yaml_file('./kube_config')
+
+        assert refresh_kube_config_loader is not None
+
+        assert refresh_kube_config_loader.current_context['name'] == 'federal-context'
+
+        context = refresh_kube_config_loader.current_context['context']
+        assert context is not None
+        assert context['cluster'] == 'horse-cluster'
+        assert context['namespace'] == 'chisel-ns'
+        assert context['user'] == 'green-user'
+
+    def test_get_api_key_with_prefix(self):
+
+        refresh_config = RefreshConfiguration()
+        refresh_config.api_key['key'] = '1234'
+        assert refresh_config is not None
+
+        api_key = refresh_config.get_api_key_with_prefix("key")
+
+        assert api_key == '1234'
+
+    def test_refresh_kube_config_loader(self):
+
+        current_context = _get_kube_config_loader_for_yaml_file('./kube_config').current_context
+
+        config_dict = {}
+        config_dict['current-context'] = 'federal-context'
+        config_dict['contexts'] = []
+        config_dict['contexts'].append(current_context)
+
+        config_dict['clusters'] = []
+
+        cluster_config = {}
+        cluster_config['api-version'] = 'v1'
+        cluster_config['server'] = 'http://cow.org:8080'
+        cluster_config['name'] = 'horse-cluster'
+        cluster_root_config = {}
+        cluster_root_config['cluster'] = cluster_config
+        cluster_root_config['name'] = 'horse-cluster'
+        config_dict['clusters'].append(cluster_root_config)
+
+        refresh_kube_config_loader = RefreshKubeConfigLoader(config_dict=config_dict)
+        refresh_kube_config_loader._user = {}
+        refresh_kube_config_loader._user['exec'] = 'test'
+
+        config_node = ConfigNode('command', 'test')
+        config_node.__dict__['apiVersion'] = '2.0'
+        config_node.__dict__['command'] = 'test'
+
+        ExecProvider.__init__ = Mock()

Review comment:
       Hi @mik-laj , sorry Im not clear ,should I change it to a patch or should I create a side_effect? Thanks




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



[GitHub] [airflow] subkanthi commented on a change in pull request #18563: Test kubernetes refresh config

Posted by GitBox <gi...@apache.org>.
subkanthi commented on a change in pull request #18563:
URL: https://github.com/apache/airflow/pull/18563#discussion_r718518978



##########
File path: tests/kubernetes/test_refresh_config.py
##########
@@ -35,3 +43,64 @@ def test_parse_timestamp_should_convert_regular_timezone_to_unix_timestamp(self)
     def test_parse_timestamp_should_throw_exception(self):
         with pytest.raises(ParserError):
             _parse_timestamp("foobar")
+
+    def test_get_kube_config_loader_for_yaml_file(self):
+        refresh_kube_config_loader = _get_kube_config_loader_for_yaml_file('./kube_config')
+
+        assert refresh_kube_config_loader is not None
+
+        assert refresh_kube_config_loader.current_context['name'] == 'federal-context'
+
+        context = refresh_kube_config_loader.current_context['context']
+        assert context is not None
+        assert context['cluster'] == 'horse-cluster'
+        assert context['namespace'] == 'chisel-ns'
+        assert context['user'] == 'green-user'
+
+    def test_get_api_key_with_prefix(self):
+
+        refresh_config = RefreshConfiguration()
+        refresh_config.api_key['key'] = '1234'
+        assert refresh_config is not None
+
+        api_key = refresh_config.get_api_key_with_prefix("key")
+
+        assert api_key == '1234'
+
+    def test_refresh_kube_config_loader(self):
+
+        current_context = _get_kube_config_loader_for_yaml_file('./kube_config').current_context
+
+        config_dict = {}
+        config_dict['current-context'] = 'federal-context'
+        config_dict['contexts'] = []
+        config_dict['contexts'].append(current_context)
+
+        config_dict['clusters'] = []
+
+        cluster_config = {}
+        cluster_config['api-version'] = 'v1'
+        cluster_config['server'] = 'http://cow.org:8080'
+        cluster_config['name'] = 'horse-cluster'
+        cluster_root_config = {}
+        cluster_root_config['cluster'] = cluster_config
+        cluster_root_config['name'] = 'horse-cluster'
+        config_dict['clusters'].append(cluster_root_config)
+
+        refresh_kube_config_loader = RefreshKubeConfigLoader(config_dict=config_dict)
+        refresh_kube_config_loader._user = {}
+        refresh_kube_config_loader._user['exec'] = 'test'
+
+        config_node = ConfigNode('command', 'test')
+        config_node.__dict__['apiVersion'] = '2.0'
+        config_node.__dict__['command'] = 'test'
+
+        ExecProvider.__init__ = Mock()
+        ExecProvider.__init__.return_value = None
+
+        ExecProvider.run = Mock()
+        ExecProvider.run.return_value = {'token': '1234'}
+
+        result = refresh_kube_config_loader._load_from_exec_plugin()

Review comment:
       For some reason with this approach, the ExecProvider is not replaced with the Mock and its calling the __init__. Trying to figure it out.
   
   `    def __init__(self, exec_config):
   
           for key in ['command', 'apiVersion']:
               if key not in exec_config:
                   raise ConfigException(
                       'exec: malformed request. missing key \'%s\'' % key)
           self.api_version = exec_config['apiVersion']
           self.args = [exec_config['command']]
           if exec_config.safe_get('args'):
               self.args.extend(exec_config['args'])
           self.env = os.environ.copy()
           if exec_config.safe_get('env'):
               additional_vars = {}
               for item in exec_config['env']:
                   name = item['name']
                   value = item['value']
                   additional_vars[name] = value
               self.env.update(additional_vars)`




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



[GitHub] [airflow] mik-laj commented on pull request #18563: Test kubernetes refresh config

Posted by GitBox <gi...@apache.org>.
mik-laj commented on pull request #18563:
URL: https://github.com/apache/airflow/pull/18563#issuecomment-939377081


   CI failed 
   ```
   tests/kubernetes/test_refresh_config.py::TestRefreshKubeConfigLoader::test_get_kube_config_loader_for_yaml_file: FileNotFoundError: [Errno 2] No such file or directory: './kube_config'
   tests/kubernetes/test_refresh_config.py::TestRefreshKubeConfigLoader::test_refresh_kube_config_loader: FileNotFoundError: [Errno 2] No such file or directory: './kube_config'
   ```
   


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



[GitHub] [airflow] subkanthi commented on a change in pull request #18563: Test kubernetes refresh config

Posted by GitBox <gi...@apache.org>.
subkanthi commented on a change in pull request #18563:
URL: https://github.com/apache/airflow/pull/18563#discussion_r725420157



##########
File path: tests/kubernetes/test_refresh_config.py
##########
@@ -35,3 +41,60 @@ def test_parse_timestamp_should_convert_regular_timezone_to_unix_timestamp(self)
     def test_parse_timestamp_should_throw_exception(self):
         with pytest.raises(ParserError):
             _parse_timestamp("foobar")
+
+    def test_get_kube_config_loader_for_yaml_file(self):
+        refresh_kube_config_loader = _get_kube_config_loader_for_yaml_file('./kube_config')
+
+        assert refresh_kube_config_loader is not None
+
+        assert refresh_kube_config_loader.current_context['name'] == 'federal-context'
+
+        context = refresh_kube_config_loader.current_context['context']
+        assert context is not None
+        assert context['cluster'] == 'horse-cluster'
+        assert context['namespace'] == 'chisel-ns'
+        assert context['user'] == 'green-user'
+
+    def test_get_api_key_with_prefix(self):
+
+        refresh_config = RefreshConfiguration()
+        refresh_config.api_key['key'] = '1234'
+        assert refresh_config is not None
+
+        api_key = refresh_config.get_api_key_with_prefix("key")
+
+        assert api_key == '1234'
+
+    @mock.patch('kubernetes.config.exec_provider.ExecProvider.__init__')
+    @mock.patch('kubernetes.config.exec_provider.ExecProvider.run')
+    def test_refresh_kube_config_loader(self, exec_provider_run, exec_provider_init):
+        current_context = _get_kube_config_loader_for_yaml_file('./kube_config').current_context
+
+        config_dict = {}
+        config_dict['current-context'] = 'federal-context'
+        config_dict['contexts'] = []
+        config_dict['contexts'].append(current_context)
+
+        config_dict['clusters'] = []
+
+        cluster_config = {}
+        cluster_config['api-version'] = 'v1'
+        cluster_config['server'] = 'http://cow.org:8080'
+        cluster_config['name'] = 'horse-cluster'
+        cluster_root_config = {}
+        cluster_root_config['cluster'] = cluster_config
+        cluster_root_config['name'] = 'horse-cluster'
+        config_dict['clusters'].append(cluster_root_config)
+
+        refresh_kube_config_loader = RefreshKubeConfigLoader(config_dict=config_dict)
+        refresh_kube_config_loader._user = {}
+
+        config_node = ConfigNode('command', 'test')
+        config_node.__dict__['apiVersion'] = '2.0'
+        config_node.__dict__['command'] = 'test'
+
+        exec_provider_init.return_value = None

Review comment:
       Moved return_value to patch, thanks.




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



[GitHub] [airflow] mik-laj commented on a change in pull request #18563: Test kubernetes refresh config

Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #18563:
URL: https://github.com/apache/airflow/pull/18563#discussion_r717209660



##########
File path: tests/kubernetes/test_refresh_config.py
##########
@@ -35,3 +43,64 @@ def test_parse_timestamp_should_convert_regular_timezone_to_unix_timestamp(self)
     def test_parse_timestamp_should_throw_exception(self):
         with pytest.raises(ParserError):
             _parse_timestamp("foobar")
+
+    def test_get_kube_config_loader_for_yaml_file(self):
+        refresh_kube_config_loader = _get_kube_config_loader_for_yaml_file('./kube_config')
+
+        assert refresh_kube_config_loader is not None
+
+        assert refresh_kube_config_loader.current_context['name'] == 'federal-context'
+
+        context = refresh_kube_config_loader.current_context['context']
+        assert context is not None
+        assert context['cluster'] == 'horse-cluster'
+        assert context['namespace'] == 'chisel-ns'
+        assert context['user'] == 'green-user'
+
+    def test_get_api_key_with_prefix(self):
+
+        refresh_config = RefreshConfiguration()
+        refresh_config.api_key['key'] = '1234'
+        assert refresh_config is not None
+
+        api_key = refresh_config.get_api_key_with_prefix("key")
+
+        assert api_key == '1234'
+
+    def test_refresh_kube_config_loader(self):
+
+        current_context = _get_kube_config_loader_for_yaml_file('./kube_config').current_context
+
+        config_dict = {}
+        config_dict['current-context'] = 'federal-context'
+        config_dict['contexts'] = []
+        config_dict['contexts'].append(current_context)
+
+        config_dict['clusters'] = []
+
+        cluster_config = {}
+        cluster_config['api-version'] = 'v1'
+        cluster_config['server'] = 'http://cow.org:8080'
+        cluster_config['name'] = 'horse-cluster'
+        cluster_root_config = {}
+        cluster_root_config['cluster'] = cluster_config
+        cluster_root_config['name'] = 'horse-cluster'
+        config_dict['clusters'].append(cluster_root_config)
+
+        refresh_kube_config_loader = RefreshKubeConfigLoader(config_dict=config_dict)
+        refresh_kube_config_loader._user = {}
+        refresh_kube_config_loader._user['exec'] = 'test'
+
+        config_node = ConfigNode('command', 'test')
+        config_node.__dict__['apiVersion'] = '2.0'
+        config_node.__dict__['command'] = 'test'
+
+        ExecProvider.__init__ = Mock()

Review comment:
       Can you use `unittest.mock` here to avoid side-effects?




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



[GitHub] [airflow] subkanthi commented on a change in pull request #18563: Test kubernetes refresh config

Posted by GitBox <gi...@apache.org>.
subkanthi commented on a change in pull request #18563:
URL: https://github.com/apache/airflow/pull/18563#discussion_r717662037



##########
File path: tests/kubernetes/test_refresh_config.py
##########
@@ -35,3 +43,64 @@ def test_parse_timestamp_should_convert_regular_timezone_to_unix_timestamp(self)
     def test_parse_timestamp_should_throw_exception(self):
         with pytest.raises(ParserError):
             _parse_timestamp("foobar")
+
+    def test_get_kube_config_loader_for_yaml_file(self):
+        refresh_kube_config_loader = _get_kube_config_loader_for_yaml_file('./kube_config')
+
+        assert refresh_kube_config_loader is not None
+
+        assert refresh_kube_config_loader.current_context['name'] == 'federal-context'
+
+        context = refresh_kube_config_loader.current_context['context']
+        assert context is not None
+        assert context['cluster'] == 'horse-cluster'
+        assert context['namespace'] == 'chisel-ns'
+        assert context['user'] == 'green-user'
+
+    def test_get_api_key_with_prefix(self):
+
+        refresh_config = RefreshConfiguration()
+        refresh_config.api_key['key'] = '1234'
+        assert refresh_config is not None
+
+        api_key = refresh_config.get_api_key_with_prefix("key")
+
+        assert api_key == '1234'
+
+    def test_refresh_kube_config_loader(self):
+
+        current_context = _get_kube_config_loader_for_yaml_file('./kube_config').current_context
+
+        config_dict = {}
+        config_dict['current-context'] = 'federal-context'
+        config_dict['contexts'] = []
+        config_dict['contexts'].append(current_context)
+
+        config_dict['clusters'] = []
+
+        cluster_config = {}
+        cluster_config['api-version'] = 'v1'
+        cluster_config['server'] = 'http://cow.org:8080'
+        cluster_config['name'] = 'horse-cluster'
+        cluster_root_config = {}
+        cluster_root_config['cluster'] = cluster_config
+        cluster_root_config['name'] = 'horse-cluster'
+        config_dict['clusters'].append(cluster_root_config)
+
+        refresh_kube_config_loader = RefreshKubeConfigLoader(config_dict=config_dict)
+        refresh_kube_config_loader._user = {}
+        refresh_kube_config_loader._user['exec'] = 'test'
+
+        config_node = ConfigNode('command', 'test')
+        config_node.__dict__['apiVersion'] = '2.0'
+        config_node.__dict__['command'] = 'test'
+
+        ExecProvider.__init__ = Mock()

Review comment:
       Hi @mik-laj , sorry Im not clear ,should I change it to a patch or should I create a side_effect? Thanks




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



[GitHub] [airflow] subkanthi commented on a change in pull request #18563: Test kubernetes refresh config

Posted by GitBox <gi...@apache.org>.
subkanthi commented on a change in pull request #18563:
URL: https://github.com/apache/airflow/pull/18563#discussion_r718518978



##########
File path: tests/kubernetes/test_refresh_config.py
##########
@@ -35,3 +43,64 @@ def test_parse_timestamp_should_convert_regular_timezone_to_unix_timestamp(self)
     def test_parse_timestamp_should_throw_exception(self):
         with pytest.raises(ParserError):
             _parse_timestamp("foobar")
+
+    def test_get_kube_config_loader_for_yaml_file(self):
+        refresh_kube_config_loader = _get_kube_config_loader_for_yaml_file('./kube_config')
+
+        assert refresh_kube_config_loader is not None
+
+        assert refresh_kube_config_loader.current_context['name'] == 'federal-context'
+
+        context = refresh_kube_config_loader.current_context['context']
+        assert context is not None
+        assert context['cluster'] == 'horse-cluster'
+        assert context['namespace'] == 'chisel-ns'
+        assert context['user'] == 'green-user'
+
+    def test_get_api_key_with_prefix(self):
+
+        refresh_config = RefreshConfiguration()
+        refresh_config.api_key['key'] = '1234'
+        assert refresh_config is not None
+
+        api_key = refresh_config.get_api_key_with_prefix("key")
+
+        assert api_key == '1234'
+
+    def test_refresh_kube_config_loader(self):
+
+        current_context = _get_kube_config_loader_for_yaml_file('./kube_config').current_context
+
+        config_dict = {}
+        config_dict['current-context'] = 'federal-context'
+        config_dict['contexts'] = []
+        config_dict['contexts'].append(current_context)
+
+        config_dict['clusters'] = []
+
+        cluster_config = {}
+        cluster_config['api-version'] = 'v1'
+        cluster_config['server'] = 'http://cow.org:8080'
+        cluster_config['name'] = 'horse-cluster'
+        cluster_root_config = {}
+        cluster_root_config['cluster'] = cluster_config
+        cluster_root_config['name'] = 'horse-cluster'
+        config_dict['clusters'].append(cluster_root_config)
+
+        refresh_kube_config_loader = RefreshKubeConfigLoader(config_dict=config_dict)
+        refresh_kube_config_loader._user = {}
+        refresh_kube_config_loader._user['exec'] = 'test'
+
+        config_node = ConfigNode('command', 'test')
+        config_node.__dict__['apiVersion'] = '2.0'
+        config_node.__dict__['command'] = 'test'
+
+        ExecProvider.__init__ = Mock()
+        ExecProvider.__init__.return_value = None
+
+        ExecProvider.run = Mock()
+        ExecProvider.run.return_value = {'token': '1234'}
+
+        result = refresh_kube_config_loader._load_from_exec_plugin()

Review comment:
       For some reason with this approach, the ExecProvider is not replaced with the Mock and its calling the __init__. Trying to figure it out.
   
   `    def __init__(self, exec_config):
           """
           exec_config must be of type ConfigNode because we depend on
           safe_get(self, key) to correctly handle optional exec provider
           config parameters.
           """
           for key in ['command', 'apiVersion']:
               if key not in exec_config:
                   raise ConfigException(
                       'exec: malformed request. missing key \'%s\'' % key)
           self.api_version = exec_config['apiVersion']
           self.args = [exec_config['command']]
           if exec_config.safe_get('args'):
               self.args.extend(exec_config['args'])
           self.env = os.environ.copy()
           if exec_config.safe_get('env'):
               additional_vars = {}
               for item in exec_config['env']:
                   name = item['name']
                   value = item['value']
                   additional_vars[name] = value
               self.env.update(additional_vars)`




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



[GitHub] [airflow] subkanthi commented on a change in pull request #18563: Test kubernetes refresh config

Posted by GitBox <gi...@apache.org>.
subkanthi commented on a change in pull request #18563:
URL: https://github.com/apache/airflow/pull/18563#discussion_r718518978



##########
File path: tests/kubernetes/test_refresh_config.py
##########
@@ -35,3 +43,64 @@ def test_parse_timestamp_should_convert_regular_timezone_to_unix_timestamp(self)
     def test_parse_timestamp_should_throw_exception(self):
         with pytest.raises(ParserError):
             _parse_timestamp("foobar")
+
+    def test_get_kube_config_loader_for_yaml_file(self):
+        refresh_kube_config_loader = _get_kube_config_loader_for_yaml_file('./kube_config')
+
+        assert refresh_kube_config_loader is not None
+
+        assert refresh_kube_config_loader.current_context['name'] == 'federal-context'
+
+        context = refresh_kube_config_loader.current_context['context']
+        assert context is not None
+        assert context['cluster'] == 'horse-cluster'
+        assert context['namespace'] == 'chisel-ns'
+        assert context['user'] == 'green-user'
+
+    def test_get_api_key_with_prefix(self):
+
+        refresh_config = RefreshConfiguration()
+        refresh_config.api_key['key'] = '1234'
+        assert refresh_config is not None
+
+        api_key = refresh_config.get_api_key_with_prefix("key")
+
+        assert api_key == '1234'
+
+    def test_refresh_kube_config_loader(self):
+
+        current_context = _get_kube_config_loader_for_yaml_file('./kube_config').current_context
+
+        config_dict = {}
+        config_dict['current-context'] = 'federal-context'
+        config_dict['contexts'] = []
+        config_dict['contexts'].append(current_context)
+
+        config_dict['clusters'] = []
+
+        cluster_config = {}
+        cluster_config['api-version'] = 'v1'
+        cluster_config['server'] = 'http://cow.org:8080'
+        cluster_config['name'] = 'horse-cluster'
+        cluster_root_config = {}
+        cluster_root_config['cluster'] = cluster_config
+        cluster_root_config['name'] = 'horse-cluster'
+        config_dict['clusters'].append(cluster_root_config)
+
+        refresh_kube_config_loader = RefreshKubeConfigLoader(config_dict=config_dict)
+        refresh_kube_config_loader._user = {}
+        refresh_kube_config_loader._user['exec'] = 'test'
+
+        config_node = ConfigNode('command', 'test')
+        config_node.__dict__['apiVersion'] = '2.0'
+        config_node.__dict__['command'] = 'test'
+
+        ExecProvider.__init__ = Mock()
+        ExecProvider.__init__.return_value = None
+
+        ExecProvider.run = Mock()
+        ExecProvider.run.return_value = {'token': '1234'}
+
+        result = refresh_kube_config_loader._load_from_exec_plugin()

Review comment:
       For some reason with this approach, the ExecProvider is not replaced with the Mock and its calling the __init__. Trying to figure it out.
   
   `    def __init__(self, exec_config):
           """
           exec_config must be of type ConfigNode because we depend on
           safe_get(self, key) to correctly handle optional exec provider
           config parameters.
           """
           for key in ['command', 'apiVersion']:
               if key not in exec_config:
                   raise ConfigException(
                       'exec: malformed request. missing key \'%s\'' % key)
           self.api_version = exec_config['apiVersion']
           self.args = [exec_config['command']]
           if exec_config.safe_get('args'):
               self.args.extend(exec_config['args'])
           self.env = os.environ.copy()
           if exec_config.safe_get('env'):
               additional_vars = {}
               for item in exec_config['env']:
                   name = item['name']
                   value = item['value']
                   additional_vars[name] = value
               self.env.update(additional_vars)`

##########
File path: tests/kubernetes/test_refresh_config.py
##########
@@ -35,3 +43,64 @@ def test_parse_timestamp_should_convert_regular_timezone_to_unix_timestamp(self)
     def test_parse_timestamp_should_throw_exception(self):
         with pytest.raises(ParserError):
             _parse_timestamp("foobar")
+
+    def test_get_kube_config_loader_for_yaml_file(self):
+        refresh_kube_config_loader = _get_kube_config_loader_for_yaml_file('./kube_config')
+
+        assert refresh_kube_config_loader is not None
+
+        assert refresh_kube_config_loader.current_context['name'] == 'federal-context'
+
+        context = refresh_kube_config_loader.current_context['context']
+        assert context is not None
+        assert context['cluster'] == 'horse-cluster'
+        assert context['namespace'] == 'chisel-ns'
+        assert context['user'] == 'green-user'
+
+    def test_get_api_key_with_prefix(self):
+
+        refresh_config = RefreshConfiguration()
+        refresh_config.api_key['key'] = '1234'
+        assert refresh_config is not None
+
+        api_key = refresh_config.get_api_key_with_prefix("key")
+
+        assert api_key == '1234'
+
+    def test_refresh_kube_config_loader(self):
+
+        current_context = _get_kube_config_loader_for_yaml_file('./kube_config').current_context
+
+        config_dict = {}
+        config_dict['current-context'] = 'federal-context'
+        config_dict['contexts'] = []
+        config_dict['contexts'].append(current_context)
+
+        config_dict['clusters'] = []
+
+        cluster_config = {}
+        cluster_config['api-version'] = 'v1'
+        cluster_config['server'] = 'http://cow.org:8080'
+        cluster_config['name'] = 'horse-cluster'
+        cluster_root_config = {}
+        cluster_root_config['cluster'] = cluster_config
+        cluster_root_config['name'] = 'horse-cluster'
+        config_dict['clusters'].append(cluster_root_config)
+
+        refresh_kube_config_loader = RefreshKubeConfigLoader(config_dict=config_dict)
+        refresh_kube_config_loader._user = {}
+        refresh_kube_config_loader._user['exec'] = 'test'
+
+        config_node = ConfigNode('command', 'test')
+        config_node.__dict__['apiVersion'] = '2.0'
+        config_node.__dict__['command'] = 'test'
+
+        ExecProvider.__init__ = Mock()
+        ExecProvider.__init__.return_value = None
+
+        ExecProvider.run = Mock()
+        ExecProvider.run.return_value = {'token': '1234'}
+
+        result = refresh_kube_config_loader._load_from_exec_plugin()

Review comment:
       For some reason with this approach, the ExecProvider is not replaced with the Mock and its calling the __init__. Trying to figure it out.
   
   `    def __init__(self, exec_config):
   
           for key in ['command', 'apiVersion']:
               if key not in exec_config:
                   raise ConfigException(
                       'exec: malformed request. missing key \'%s\'' % key)
           self.api_version = exec_config['apiVersion']
           self.args = [exec_config['command']]
           if exec_config.safe_get('args'):
               self.args.extend(exec_config['args'])
           self.env = os.environ.copy()
           if exec_config.safe_get('env'):
               additional_vars = {}
               for item in exec_config['env']:
                   name = item['name']
                   value = item['value']
                   additional_vars[name] = value
               self.env.update(additional_vars)`

##########
File path: tests/kubernetes/test_refresh_config.py
##########
@@ -35,3 +43,64 @@ def test_parse_timestamp_should_convert_regular_timezone_to_unix_timestamp(self)
     def test_parse_timestamp_should_throw_exception(self):
         with pytest.raises(ParserError):
             _parse_timestamp("foobar")
+
+    def test_get_kube_config_loader_for_yaml_file(self):
+        refresh_kube_config_loader = _get_kube_config_loader_for_yaml_file('./kube_config')
+
+        assert refresh_kube_config_loader is not None
+
+        assert refresh_kube_config_loader.current_context['name'] == 'federal-context'
+
+        context = refresh_kube_config_loader.current_context['context']
+        assert context is not None
+        assert context['cluster'] == 'horse-cluster'
+        assert context['namespace'] == 'chisel-ns'
+        assert context['user'] == 'green-user'
+
+    def test_get_api_key_with_prefix(self):
+
+        refresh_config = RefreshConfiguration()
+        refresh_config.api_key['key'] = '1234'
+        assert refresh_config is not None
+
+        api_key = refresh_config.get_api_key_with_prefix("key")
+
+        assert api_key == '1234'
+
+    def test_refresh_kube_config_loader(self):
+
+        current_context = _get_kube_config_loader_for_yaml_file('./kube_config').current_context
+
+        config_dict = {}
+        config_dict['current-context'] = 'federal-context'
+        config_dict['contexts'] = []
+        config_dict['contexts'].append(current_context)
+
+        config_dict['clusters'] = []
+
+        cluster_config = {}
+        cluster_config['api-version'] = 'v1'
+        cluster_config['server'] = 'http://cow.org:8080'
+        cluster_config['name'] = 'horse-cluster'
+        cluster_root_config = {}
+        cluster_root_config['cluster'] = cluster_config
+        cluster_root_config['name'] = 'horse-cluster'
+        config_dict['clusters'].append(cluster_root_config)
+
+        refresh_kube_config_loader = RefreshKubeConfigLoader(config_dict=config_dict)
+        refresh_kube_config_loader._user = {}
+        refresh_kube_config_loader._user['exec'] = 'test'
+
+        config_node = ConfigNode('command', 'test')
+        config_node.__dict__['apiVersion'] = '2.0'
+        config_node.__dict__['command'] = 'test'
+
+        ExecProvider.__init__ = Mock()
+        ExecProvider.__init__.return_value = None
+
+        ExecProvider.run = Mock()
+        ExecProvider.run.return_value = {'token': '1234'}
+
+        result = refresh_kube_config_loader._load_from_exec_plugin()

Review comment:
       @mik-laj changed to use decorators, please let me know if its fine, I couldnt figure out the syntax for mocking the constructor in context manager.




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



[GitHub] [airflow] subkanthi commented on a change in pull request #18563: Test kubernetes refresh config

Posted by GitBox <gi...@apache.org>.
subkanthi commented on a change in pull request #18563:
URL: https://github.com/apache/airflow/pull/18563#discussion_r718626461



##########
File path: tests/kubernetes/test_refresh_config.py
##########
@@ -35,3 +43,64 @@ def test_parse_timestamp_should_convert_regular_timezone_to_unix_timestamp(self)
     def test_parse_timestamp_should_throw_exception(self):
         with pytest.raises(ParserError):
             _parse_timestamp("foobar")
+
+    def test_get_kube_config_loader_for_yaml_file(self):
+        refresh_kube_config_loader = _get_kube_config_loader_for_yaml_file('./kube_config')
+
+        assert refresh_kube_config_loader is not None
+
+        assert refresh_kube_config_loader.current_context['name'] == 'federal-context'
+
+        context = refresh_kube_config_loader.current_context['context']
+        assert context is not None
+        assert context['cluster'] == 'horse-cluster'
+        assert context['namespace'] == 'chisel-ns'
+        assert context['user'] == 'green-user'
+
+    def test_get_api_key_with_prefix(self):
+
+        refresh_config = RefreshConfiguration()
+        refresh_config.api_key['key'] = '1234'
+        assert refresh_config is not None
+
+        api_key = refresh_config.get_api_key_with_prefix("key")
+
+        assert api_key == '1234'
+
+    def test_refresh_kube_config_loader(self):
+
+        current_context = _get_kube_config_loader_for_yaml_file('./kube_config').current_context
+
+        config_dict = {}
+        config_dict['current-context'] = 'federal-context'
+        config_dict['contexts'] = []
+        config_dict['contexts'].append(current_context)
+
+        config_dict['clusters'] = []
+
+        cluster_config = {}
+        cluster_config['api-version'] = 'v1'
+        cluster_config['server'] = 'http://cow.org:8080'
+        cluster_config['name'] = 'horse-cluster'
+        cluster_root_config = {}
+        cluster_root_config['cluster'] = cluster_config
+        cluster_root_config['name'] = 'horse-cluster'
+        config_dict['clusters'].append(cluster_root_config)
+
+        refresh_kube_config_loader = RefreshKubeConfigLoader(config_dict=config_dict)
+        refresh_kube_config_loader._user = {}
+        refresh_kube_config_loader._user['exec'] = 'test'
+
+        config_node = ConfigNode('command', 'test')
+        config_node.__dict__['apiVersion'] = '2.0'
+        config_node.__dict__['command'] = 'test'
+
+        ExecProvider.__init__ = Mock()
+        ExecProvider.__init__.return_value = None
+
+        ExecProvider.run = Mock()
+        ExecProvider.run.return_value = {'token': '1234'}
+
+        result = refresh_kube_config_loader._load_from_exec_plugin()

Review comment:
       @mik-laj changed to use decorators, please let me know if its fine, I couldnt figure out the syntax for mocking the constructor in context manager.




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



[GitHub] [airflow] mik-laj commented on a change in pull request #18563: Test kubernetes refresh config

Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #18563:
URL: https://github.com/apache/airflow/pull/18563#discussion_r725551133



##########
File path: tests/kubernetes/test_refresh_config.py
##########
@@ -35,3 +41,59 @@ def test_parse_timestamp_should_convert_regular_timezone_to_unix_timestamp(self)
     def test_parse_timestamp_should_throw_exception(self):
         with pytest.raises(ParserError):
             _parse_timestamp("foobar")
+
+    def test_get_kube_config_loader_for_yaml_file(self):
+        refresh_kube_config_loader = _get_kube_config_loader_for_yaml_file('./kube_config')

Review comment:
       It seems the CI cannot recognize the relative path. Can you use an absolute path built from __file__? It should be something similar to code below:
   ```python
   ROOT_PROJECT_DIR = os.path.abspath(
       os.path.join(os.path.dirname(os.path.realpath(__file__)), os.pardir, os.pardir, os.pardir)
   )
   ```




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



[GitHub] [airflow] uranusjr commented on a change in pull request #18563: Test kubernetes refresh config

Posted by GitBox <gi...@apache.org>.
uranusjr commented on a change in pull request #18563:
URL: https://github.com/apache/airflow/pull/18563#discussion_r723789246



##########
File path: tests/kubernetes/test_refresh_config.py
##########
@@ -35,3 +41,60 @@ def test_parse_timestamp_should_convert_regular_timezone_to_unix_timestamp(self)
     def test_parse_timestamp_should_throw_exception(self):
         with pytest.raises(ParserError):
             _parse_timestamp("foobar")
+
+    def test_get_kube_config_loader_for_yaml_file(self):
+        refresh_kube_config_loader = _get_kube_config_loader_for_yaml_file('./kube_config')
+
+        assert refresh_kube_config_loader is not None
+
+        assert refresh_kube_config_loader.current_context['name'] == 'federal-context'
+
+        context = refresh_kube_config_loader.current_context['context']
+        assert context is not None
+        assert context['cluster'] == 'horse-cluster'
+        assert context['namespace'] == 'chisel-ns'
+        assert context['user'] == 'green-user'
+
+    def test_get_api_key_with_prefix(self):
+
+        refresh_config = RefreshConfiguration()
+        refresh_config.api_key['key'] = '1234'
+        assert refresh_config is not None
+
+        api_key = refresh_config.get_api_key_with_prefix("key")
+
+        assert api_key == '1234'
+
+    @mock.patch('kubernetes.config.exec_provider.ExecProvider.__init__')
+    @mock.patch('kubernetes.config.exec_provider.ExecProvider.run')
+    def test_refresh_kube_config_loader(self, exec_provider_run, exec_provider_init):
+        current_context = _get_kube_config_loader_for_yaml_file('./kube_config').current_context
+
+        config_dict = {}
+        config_dict['current-context'] = 'federal-context'
+        config_dict['contexts'] = []
+        config_dict['contexts'].append(current_context)
+
+        config_dict['clusters'] = []
+
+        cluster_config = {}
+        cluster_config['api-version'] = 'v1'
+        cluster_config['server'] = 'http://cow.org:8080'
+        cluster_config['name'] = 'horse-cluster'
+        cluster_root_config = {}
+        cluster_root_config['cluster'] = cluster_config
+        cluster_root_config['name'] = 'horse-cluster'
+        config_dict['clusters'].append(cluster_root_config)
+
+        refresh_kube_config_loader = RefreshKubeConfigLoader(config_dict=config_dict)
+        refresh_kube_config_loader._user = {}
+
+        config_node = ConfigNode('command', 'test')
+        config_node.__dict__['apiVersion'] = '2.0'
+        config_node.__dict__['command'] = 'test'
+
+        exec_provider_init.return_value = None

Review comment:
       You can do this in the `patch` call by adding `return_value=None`. (Same for `exec_provider_run.return_value` below.)




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



[GitHub] [airflow] mik-laj commented on a change in pull request #18563: Test kubernetes refresh config

Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #18563:
URL: https://github.com/apache/airflow/pull/18563#discussion_r717209660



##########
File path: tests/kubernetes/test_refresh_config.py
##########
@@ -35,3 +43,64 @@ def test_parse_timestamp_should_convert_regular_timezone_to_unix_timestamp(self)
     def test_parse_timestamp_should_throw_exception(self):
         with pytest.raises(ParserError):
             _parse_timestamp("foobar")
+
+    def test_get_kube_config_loader_for_yaml_file(self):
+        refresh_kube_config_loader = _get_kube_config_loader_for_yaml_file('./kube_config')
+
+        assert refresh_kube_config_loader is not None
+
+        assert refresh_kube_config_loader.current_context['name'] == 'federal-context'
+
+        context = refresh_kube_config_loader.current_context['context']
+        assert context is not None
+        assert context['cluster'] == 'horse-cluster'
+        assert context['namespace'] == 'chisel-ns'
+        assert context['user'] == 'green-user'
+
+    def test_get_api_key_with_prefix(self):
+
+        refresh_config = RefreshConfiguration()
+        refresh_config.api_key['key'] = '1234'
+        assert refresh_config is not None
+
+        api_key = refresh_config.get_api_key_with_prefix("key")
+
+        assert api_key == '1234'
+
+    def test_refresh_kube_config_loader(self):
+
+        current_context = _get_kube_config_loader_for_yaml_file('./kube_config').current_context
+
+        config_dict = {}
+        config_dict['current-context'] = 'federal-context'
+        config_dict['contexts'] = []
+        config_dict['contexts'].append(current_context)
+
+        config_dict['clusters'] = []
+
+        cluster_config = {}
+        cluster_config['api-version'] = 'v1'
+        cluster_config['server'] = 'http://cow.org:8080'
+        cluster_config['name'] = 'horse-cluster'
+        cluster_root_config = {}
+        cluster_root_config['cluster'] = cluster_config
+        cluster_root_config['name'] = 'horse-cluster'
+        config_dict['clusters'].append(cluster_root_config)
+
+        refresh_kube_config_loader = RefreshKubeConfigLoader(config_dict=config_dict)
+        refresh_kube_config_loader._user = {}
+        refresh_kube_config_loader._user['exec'] = 'test'
+
+        config_node = ConfigNode('command', 'test')
+        config_node.__dict__['apiVersion'] = '2.0'
+        config_node.__dict__['command'] = 'test'
+
+        ExecProvider.__init__ = Mock()

Review comment:
       Can you use `unittest.mock` here to avoid side-effects?

##########
File path: tests/kubernetes/test_refresh_config.py
##########
@@ -35,3 +43,64 @@ def test_parse_timestamp_should_convert_regular_timezone_to_unix_timestamp(self)
     def test_parse_timestamp_should_throw_exception(self):
         with pytest.raises(ParserError):
             _parse_timestamp("foobar")
+
+    def test_get_kube_config_loader_for_yaml_file(self):
+        refresh_kube_config_loader = _get_kube_config_loader_for_yaml_file('./kube_config')
+
+        assert refresh_kube_config_loader is not None
+
+        assert refresh_kube_config_loader.current_context['name'] == 'federal-context'
+
+        context = refresh_kube_config_loader.current_context['context']
+        assert context is not None
+        assert context['cluster'] == 'horse-cluster'
+        assert context['namespace'] == 'chisel-ns'
+        assert context['user'] == 'green-user'
+
+    def test_get_api_key_with_prefix(self):
+
+        refresh_config = RefreshConfiguration()
+        refresh_config.api_key['key'] = '1234'
+        assert refresh_config is not None
+
+        api_key = refresh_config.get_api_key_with_prefix("key")
+
+        assert api_key == '1234'
+
+    def test_refresh_kube_config_loader(self):
+
+        current_context = _get_kube_config_loader_for_yaml_file('./kube_config').current_context
+
+        config_dict = {}
+        config_dict['current-context'] = 'federal-context'
+        config_dict['contexts'] = []
+        config_dict['contexts'].append(current_context)
+
+        config_dict['clusters'] = []
+
+        cluster_config = {}
+        cluster_config['api-version'] = 'v1'
+        cluster_config['server'] = 'http://cow.org:8080'
+        cluster_config['name'] = 'horse-cluster'
+        cluster_root_config = {}
+        cluster_root_config['cluster'] = cluster_config
+        cluster_root_config['name'] = 'horse-cluster'
+        config_dict['clusters'].append(cluster_root_config)
+
+        refresh_kube_config_loader = RefreshKubeConfigLoader(config_dict=config_dict)
+        refresh_kube_config_loader._user = {}
+        refresh_kube_config_loader._user['exec'] = 'test'
+
+        config_node = ConfigNode('command', 'test')
+        config_node.__dict__['apiVersion'] = '2.0'
+        config_node.__dict__['command'] = 'test'
+
+        ExecProvider.__init__ = Mock()

Review comment:
       You should use contextmanager/decoraator here. This way we are sure that the overwritten method will revert to its original form.

##########
File path: tests/kubernetes/test_refresh_config.py
##########
@@ -35,3 +43,64 @@ def test_parse_timestamp_should_convert_regular_timezone_to_unix_timestamp(self)
     def test_parse_timestamp_should_throw_exception(self):
         with pytest.raises(ParserError):
             _parse_timestamp("foobar")
+
+    def test_get_kube_config_loader_for_yaml_file(self):
+        refresh_kube_config_loader = _get_kube_config_loader_for_yaml_file('./kube_config')
+
+        assert refresh_kube_config_loader is not None
+
+        assert refresh_kube_config_loader.current_context['name'] == 'federal-context'
+
+        context = refresh_kube_config_loader.current_context['context']
+        assert context is not None
+        assert context['cluster'] == 'horse-cluster'
+        assert context['namespace'] == 'chisel-ns'
+        assert context['user'] == 'green-user'
+
+    def test_get_api_key_with_prefix(self):
+
+        refresh_config = RefreshConfiguration()
+        refresh_config.api_key['key'] = '1234'
+        assert refresh_config is not None
+
+        api_key = refresh_config.get_api_key_with_prefix("key")
+
+        assert api_key == '1234'
+
+    def test_refresh_kube_config_loader(self):
+
+        current_context = _get_kube_config_loader_for_yaml_file('./kube_config').current_context
+
+        config_dict = {}
+        config_dict['current-context'] = 'federal-context'
+        config_dict['contexts'] = []
+        config_dict['contexts'].append(current_context)
+
+        config_dict['clusters'] = []
+
+        cluster_config = {}
+        cluster_config['api-version'] = 'v1'
+        cluster_config['server'] = 'http://cow.org:8080'
+        cluster_config['name'] = 'horse-cluster'
+        cluster_root_config = {}
+        cluster_root_config['cluster'] = cluster_config
+        cluster_root_config['name'] = 'horse-cluster'
+        config_dict['clusters'].append(cluster_root_config)
+
+        refresh_kube_config_loader = RefreshKubeConfigLoader(config_dict=config_dict)
+        refresh_kube_config_loader._user = {}
+        refresh_kube_config_loader._user['exec'] = 'test'
+
+        config_node = ConfigNode('command', 'test')
+        config_node.__dict__['apiVersion'] = '2.0'
+        config_node.__dict__['command'] = 'test'
+
+        ExecProvider.__init__ = Mock()
+        ExecProvider.__init__.return_value = None
+
+        ExecProvider.run = Mock()
+        ExecProvider.run.return_value = {'token': '1234'}
+
+        result = refresh_kube_config_loader._load_from_exec_plugin()

Review comment:
       ```suggestion
           with unittest.mock('kubernetes.config.exec_provider.ExecProvider') as mock_exec_provider:
                   mock_exec_provider.return_value.run.return_value = {'token': '1234'}
                   
                   result = refresh_kube_config_loader._load_from_exec_plugin()
   
           assert result is not None
   ```

##########
File path: tests/kubernetes/test_refresh_config.py
##########
@@ -35,3 +43,64 @@ def test_parse_timestamp_should_convert_regular_timezone_to_unix_timestamp(self)
     def test_parse_timestamp_should_throw_exception(self):
         with pytest.raises(ParserError):
             _parse_timestamp("foobar")
+
+    def test_get_kube_config_loader_for_yaml_file(self):
+        refresh_kube_config_loader = _get_kube_config_loader_for_yaml_file('./kube_config')
+
+        assert refresh_kube_config_loader is not None
+
+        assert refresh_kube_config_loader.current_context['name'] == 'federal-context'
+
+        context = refresh_kube_config_loader.current_context['context']
+        assert context is not None
+        assert context['cluster'] == 'horse-cluster'
+        assert context['namespace'] == 'chisel-ns'
+        assert context['user'] == 'green-user'
+
+    def test_get_api_key_with_prefix(self):
+
+        refresh_config = RefreshConfiguration()
+        refresh_config.api_key['key'] = '1234'
+        assert refresh_config is not None
+
+        api_key = refresh_config.get_api_key_with_prefix("key")
+
+        assert api_key == '1234'
+
+    def test_refresh_kube_config_loader(self):
+
+        current_context = _get_kube_config_loader_for_yaml_file('./kube_config').current_context
+
+        config_dict = {}
+        config_dict['current-context'] = 'federal-context'
+        config_dict['contexts'] = []
+        config_dict['contexts'].append(current_context)
+
+        config_dict['clusters'] = []
+
+        cluster_config = {}
+        cluster_config['api-version'] = 'v1'
+        cluster_config['server'] = 'http://cow.org:8080'
+        cluster_config['name'] = 'horse-cluster'
+        cluster_root_config = {}
+        cluster_root_config['cluster'] = cluster_config
+        cluster_root_config['name'] = 'horse-cluster'
+        config_dict['clusters'].append(cluster_root_config)
+
+        refresh_kube_config_loader = RefreshKubeConfigLoader(config_dict=config_dict)
+        refresh_kube_config_loader._user = {}
+        refresh_kube_config_loader._user['exec'] = 'test'
+
+        config_node = ConfigNode('command', 'test')
+        config_node.__dict__['apiVersion'] = '2.0'
+        config_node.__dict__['command'] = 'test'
+
+        ExecProvider.__init__ = Mock()
+        ExecProvider.__init__.return_value = None
+
+        ExecProvider.run = Mock()
+        ExecProvider.run.return_value = {'token': '1234'}
+
+        result = refresh_kube_config_loader._load_from_exec_plugin()

Review comment:
       ```suggestion
           with unittest.mock.patch('kubernetes.config.exec_provider.ExecProvider') as mock_exec_provider:
                   mock_exec_provider.return_value.run.return_value = {'token': '1234'}
                   
                   result = refresh_kube_config_loader._load_from_exec_plugin()
   
           assert result is not None
   ```




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



[GitHub] [airflow] uranusjr commented on a change in pull request #18563: Test kubernetes refresh config

Posted by GitBox <gi...@apache.org>.
uranusjr commented on a change in pull request #18563:
URL: https://github.com/apache/airflow/pull/18563#discussion_r723789246



##########
File path: tests/kubernetes/test_refresh_config.py
##########
@@ -35,3 +41,60 @@ def test_parse_timestamp_should_convert_regular_timezone_to_unix_timestamp(self)
     def test_parse_timestamp_should_throw_exception(self):
         with pytest.raises(ParserError):
             _parse_timestamp("foobar")
+
+    def test_get_kube_config_loader_for_yaml_file(self):
+        refresh_kube_config_loader = _get_kube_config_loader_for_yaml_file('./kube_config')
+
+        assert refresh_kube_config_loader is not None
+
+        assert refresh_kube_config_loader.current_context['name'] == 'federal-context'
+
+        context = refresh_kube_config_loader.current_context['context']
+        assert context is not None
+        assert context['cluster'] == 'horse-cluster'
+        assert context['namespace'] == 'chisel-ns'
+        assert context['user'] == 'green-user'
+
+    def test_get_api_key_with_prefix(self):
+
+        refresh_config = RefreshConfiguration()
+        refresh_config.api_key['key'] = '1234'
+        assert refresh_config is not None
+
+        api_key = refresh_config.get_api_key_with_prefix("key")
+
+        assert api_key == '1234'
+
+    @mock.patch('kubernetes.config.exec_provider.ExecProvider.__init__')
+    @mock.patch('kubernetes.config.exec_provider.ExecProvider.run')
+    def test_refresh_kube_config_loader(self, exec_provider_run, exec_provider_init):
+        current_context = _get_kube_config_loader_for_yaml_file('./kube_config').current_context
+
+        config_dict = {}
+        config_dict['current-context'] = 'federal-context'
+        config_dict['contexts'] = []
+        config_dict['contexts'].append(current_context)
+
+        config_dict['clusters'] = []
+
+        cluster_config = {}
+        cluster_config['api-version'] = 'v1'
+        cluster_config['server'] = 'http://cow.org:8080'
+        cluster_config['name'] = 'horse-cluster'
+        cluster_root_config = {}
+        cluster_root_config['cluster'] = cluster_config
+        cluster_root_config['name'] = 'horse-cluster'
+        config_dict['clusters'].append(cluster_root_config)
+
+        refresh_kube_config_loader = RefreshKubeConfigLoader(config_dict=config_dict)
+        refresh_kube_config_loader._user = {}
+
+        config_node = ConfigNode('command', 'test')
+        config_node.__dict__['apiVersion'] = '2.0'
+        config_node.__dict__['command'] = 'test'
+
+        exec_provider_init.return_value = None

Review comment:
       You can do this in the `patch` call by adding `return_value=None`. (Same for `exec_provider_run.return_value` below.)




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



[GitHub] [airflow] mik-laj commented on a change in pull request #18563: Test kubernetes refresh config

Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #18563:
URL: https://github.com/apache/airflow/pull/18563#discussion_r717840461



##########
File path: tests/kubernetes/test_refresh_config.py
##########
@@ -35,3 +43,64 @@ def test_parse_timestamp_should_convert_regular_timezone_to_unix_timestamp(self)
     def test_parse_timestamp_should_throw_exception(self):
         with pytest.raises(ParserError):
             _parse_timestamp("foobar")
+
+    def test_get_kube_config_loader_for_yaml_file(self):
+        refresh_kube_config_loader = _get_kube_config_loader_for_yaml_file('./kube_config')
+
+        assert refresh_kube_config_loader is not None
+
+        assert refresh_kube_config_loader.current_context['name'] == 'federal-context'
+
+        context = refresh_kube_config_loader.current_context['context']
+        assert context is not None
+        assert context['cluster'] == 'horse-cluster'
+        assert context['namespace'] == 'chisel-ns'
+        assert context['user'] == 'green-user'
+
+    def test_get_api_key_with_prefix(self):
+
+        refresh_config = RefreshConfiguration()
+        refresh_config.api_key['key'] = '1234'
+        assert refresh_config is not None
+
+        api_key = refresh_config.get_api_key_with_prefix("key")
+
+        assert api_key == '1234'
+
+    def test_refresh_kube_config_loader(self):
+
+        current_context = _get_kube_config_loader_for_yaml_file('./kube_config').current_context
+
+        config_dict = {}
+        config_dict['current-context'] = 'federal-context'
+        config_dict['contexts'] = []
+        config_dict['contexts'].append(current_context)
+
+        config_dict['clusters'] = []
+
+        cluster_config = {}
+        cluster_config['api-version'] = 'v1'
+        cluster_config['server'] = 'http://cow.org:8080'
+        cluster_config['name'] = 'horse-cluster'
+        cluster_root_config = {}
+        cluster_root_config['cluster'] = cluster_config
+        cluster_root_config['name'] = 'horse-cluster'
+        config_dict['clusters'].append(cluster_root_config)
+
+        refresh_kube_config_loader = RefreshKubeConfigLoader(config_dict=config_dict)
+        refresh_kube_config_loader._user = {}
+        refresh_kube_config_loader._user['exec'] = 'test'
+
+        config_node = ConfigNode('command', 'test')
+        config_node.__dict__['apiVersion'] = '2.0'
+        config_node.__dict__['command'] = 'test'
+
+        ExecProvider.__init__ = Mock()
+        ExecProvider.__init__.return_value = None
+
+        ExecProvider.run = Mock()
+        ExecProvider.run.return_value = {'token': '1234'}
+
+        result = refresh_kube_config_loader._load_from_exec_plugin()

Review comment:
       ```suggestion
           with unittest.mock('kubernetes.config.exec_provider.ExecProvider') as mock_exec_provider:
                   mock_exec_provider.return_value.run.return_value = {'token': '1234'}
                   
                   result = refresh_kube_config_loader._load_from_exec_plugin()
   
           assert result is not None
   ```

##########
File path: tests/kubernetes/test_refresh_config.py
##########
@@ -35,3 +43,64 @@ def test_parse_timestamp_should_convert_regular_timezone_to_unix_timestamp(self)
     def test_parse_timestamp_should_throw_exception(self):
         with pytest.raises(ParserError):
             _parse_timestamp("foobar")
+
+    def test_get_kube_config_loader_for_yaml_file(self):
+        refresh_kube_config_loader = _get_kube_config_loader_for_yaml_file('./kube_config')
+
+        assert refresh_kube_config_loader is not None
+
+        assert refresh_kube_config_loader.current_context['name'] == 'federal-context'
+
+        context = refresh_kube_config_loader.current_context['context']
+        assert context is not None
+        assert context['cluster'] == 'horse-cluster'
+        assert context['namespace'] == 'chisel-ns'
+        assert context['user'] == 'green-user'
+
+    def test_get_api_key_with_prefix(self):
+
+        refresh_config = RefreshConfiguration()
+        refresh_config.api_key['key'] = '1234'
+        assert refresh_config is not None
+
+        api_key = refresh_config.get_api_key_with_prefix("key")
+
+        assert api_key == '1234'
+
+    def test_refresh_kube_config_loader(self):
+
+        current_context = _get_kube_config_loader_for_yaml_file('./kube_config').current_context
+
+        config_dict = {}
+        config_dict['current-context'] = 'federal-context'
+        config_dict['contexts'] = []
+        config_dict['contexts'].append(current_context)
+
+        config_dict['clusters'] = []
+
+        cluster_config = {}
+        cluster_config['api-version'] = 'v1'
+        cluster_config['server'] = 'http://cow.org:8080'
+        cluster_config['name'] = 'horse-cluster'
+        cluster_root_config = {}
+        cluster_root_config['cluster'] = cluster_config
+        cluster_root_config['name'] = 'horse-cluster'
+        config_dict['clusters'].append(cluster_root_config)
+
+        refresh_kube_config_loader = RefreshKubeConfigLoader(config_dict=config_dict)
+        refresh_kube_config_loader._user = {}
+        refresh_kube_config_loader._user['exec'] = 'test'
+
+        config_node = ConfigNode('command', 'test')
+        config_node.__dict__['apiVersion'] = '2.0'
+        config_node.__dict__['command'] = 'test'
+
+        ExecProvider.__init__ = Mock()
+        ExecProvider.__init__.return_value = None
+
+        ExecProvider.run = Mock()
+        ExecProvider.run.return_value = {'token': '1234'}
+
+        result = refresh_kube_config_loader._load_from_exec_plugin()

Review comment:
       ```suggestion
           with unittest.mock.patch('kubernetes.config.exec_provider.ExecProvider') as mock_exec_provider:
                   mock_exec_provider.return_value.run.return_value = {'token': '1234'}
                   
                   result = refresh_kube_config_loader._load_from_exec_plugin()
   
           assert result is not None
   ```




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



[GitHub] [airflow] mik-laj commented on a change in pull request #18563: Test kubernetes refresh config

Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #18563:
URL: https://github.com/apache/airflow/pull/18563#discussion_r717671040



##########
File path: tests/kubernetes/test_refresh_config.py
##########
@@ -35,3 +43,64 @@ def test_parse_timestamp_should_convert_regular_timezone_to_unix_timestamp(self)
     def test_parse_timestamp_should_throw_exception(self):
         with pytest.raises(ParserError):
             _parse_timestamp("foobar")
+
+    def test_get_kube_config_loader_for_yaml_file(self):
+        refresh_kube_config_loader = _get_kube_config_loader_for_yaml_file('./kube_config')
+
+        assert refresh_kube_config_loader is not None
+
+        assert refresh_kube_config_loader.current_context['name'] == 'federal-context'
+
+        context = refresh_kube_config_loader.current_context['context']
+        assert context is not None
+        assert context['cluster'] == 'horse-cluster'
+        assert context['namespace'] == 'chisel-ns'
+        assert context['user'] == 'green-user'
+
+    def test_get_api_key_with_prefix(self):
+
+        refresh_config = RefreshConfiguration()
+        refresh_config.api_key['key'] = '1234'
+        assert refresh_config is not None
+
+        api_key = refresh_config.get_api_key_with_prefix("key")
+
+        assert api_key == '1234'
+
+    def test_refresh_kube_config_loader(self):
+
+        current_context = _get_kube_config_loader_for_yaml_file('./kube_config').current_context
+
+        config_dict = {}
+        config_dict['current-context'] = 'federal-context'
+        config_dict['contexts'] = []
+        config_dict['contexts'].append(current_context)
+
+        config_dict['clusters'] = []
+
+        cluster_config = {}
+        cluster_config['api-version'] = 'v1'
+        cluster_config['server'] = 'http://cow.org:8080'
+        cluster_config['name'] = 'horse-cluster'
+        cluster_root_config = {}
+        cluster_root_config['cluster'] = cluster_config
+        cluster_root_config['name'] = 'horse-cluster'
+        config_dict['clusters'].append(cluster_root_config)
+
+        refresh_kube_config_loader = RefreshKubeConfigLoader(config_dict=config_dict)
+        refresh_kube_config_loader._user = {}
+        refresh_kube_config_loader._user['exec'] = 'test'
+
+        config_node = ConfigNode('command', 'test')
+        config_node.__dict__['apiVersion'] = '2.0'
+        config_node.__dict__['command'] = 'test'
+
+        ExecProvider.__init__ = Mock()

Review comment:
       You should use contextmanager/decoraator here. This way we are sure that the overwritten method will revert to its original form.




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



[GitHub] [airflow] mik-laj merged pull request #18563: Test kubernetes refresh config

Posted by GitBox <gi...@apache.org>.
mik-laj merged pull request #18563:
URL: https://github.com/apache/airflow/pull/18563


   


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



[GitHub] [airflow] mik-laj commented on a change in pull request #18563: Test kubernetes refresh config

Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #18563:
URL: https://github.com/apache/airflow/pull/18563#discussion_r725551133



##########
File path: tests/kubernetes/test_refresh_config.py
##########
@@ -35,3 +41,59 @@ def test_parse_timestamp_should_convert_regular_timezone_to_unix_timestamp(self)
     def test_parse_timestamp_should_throw_exception(self):
         with pytest.raises(ParserError):
             _parse_timestamp("foobar")
+
+    def test_get_kube_config_loader_for_yaml_file(self):
+        refresh_kube_config_loader = _get_kube_config_loader_for_yaml_file('./kube_config')

Review comment:
       It seems the CI cannot recognize the relative path. Can you use an absolute path built from __file__? It should be something similar to code below:
   ```
   ROOT_PROJECT_DIR = os.path.abspath(
       os.path.join(os.path.dirname(os.path.realpath(__file__)), os.pardir, os.pardir, os.pardir)
   )
   ```




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



[GitHub] [airflow] subkanthi commented on a change in pull request #18563: Test kubernetes refresh config

Posted by GitBox <gi...@apache.org>.
subkanthi commented on a change in pull request #18563:
URL: https://github.com/apache/airflow/pull/18563#discussion_r728344399



##########
File path: tests/kubernetes/test_refresh_config.py
##########
@@ -35,3 +41,59 @@ def test_parse_timestamp_should_convert_regular_timezone_to_unix_timestamp(self)
     def test_parse_timestamp_should_throw_exception(self):
         with pytest.raises(ParserError):
             _parse_timestamp("foobar")
+
+    def test_get_kube_config_loader_for_yaml_file(self):
+        refresh_kube_config_loader = _get_kube_config_loader_for_yaml_file('./kube_config')

Review comment:
       @mik-laj , looks good now.




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