eladkal commented on a change in pull request #16365:
URL: https://github.com/apache/airflow/pull/16365#discussion_r654885676
##########
File path: tests/providers/tableau/hooks/test_tableau.py
##########
@@ -52,6 +52,24 @@ def setUp(self):
extra='{"token_name": "my_token", "personal_access_token":
"my_personal_access_token"}',
)
)
+ db.merge_conn(
+ models.Connection(
+ conn_id='tableau_test_ssl_connection',
+ conn_type='tableau',
+ host='tableau',
+ login='user',
+ password='password',
+ extra='{"verify": "my_cert", "cert": "my_client_cert"}',
Review comment:
my_cert isn't a valid option for verify it's a bit confusing.
##########
File path: tests/providers/tableau/hooks/test_tableau.py
##########
@@ -87,6 +105,46 @@ def test_get_conn_auth_via_token_and_site_in_init(self,
mock_server, mock_tablea
)
mock_server.return_value.auth.sign_out.assert_called_once_with()
+ @patch('airflow.providers.tableau.hooks.tableau.TableauAuth')
+ @patch('airflow.providers.tableau.hooks.tableau.Server')
+ def test_get_conn_ssl(self, mock_server, mock_tableau_auth):
+ """
+ Test get conn with SSL parameters
Review comment:
```suggestion
Test get conn with SSL disabled parameters
```
##########
File path: tests/providers/tableau/hooks/test_tableau.py
##########
@@ -52,6 +52,24 @@ def setUp(self):
extra='{"token_name": "my_token", "personal_access_token":
"my_personal_access_token"}',
)
)
+ db.merge_conn(
+ models.Connection(
+ conn_id='tableau_test_ssl_connection',
Review comment:
```suggestion
conn_id='tableau_test_ssl_false_connection',
```
##########
File path: airflow/providers/tableau/hooks/tableau.py
##########
@@ -61,7 +57,10 @@ def __init__(self, site_id: Optional[str] = None,
tableau_conn_id: str = default
self.tableau_conn_id = tableau_conn_id
self.conn = self.get_connection(self.tableau_conn_id)
self.site_id = site_id or self.conn.extra_dejson.get('site_id', '')
- self.server = Server(self.conn.host, use_server_version=True)
+ self.server = Server(self.conn.host)
+ self.server.add_http_options(options_dict={'verify':
self.conn.extra_dejson.get('verify', True),
Review comment:
When user set to False I'm not sure if this will give us `{'verify':
False}`. I think this will be `{'verify': 'False'}` because there is no
conversion of string to bool
--
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:
[email protected]
