Skip to content

Commit

Permalink
feat: Require optional parameters to be keyword-specified
Browse files Browse the repository at this point in the history
BREAKING CHANGE: Keyword-specific optional parameters are new in Python3
  • Loading branch information
rmkeezer committed Apr 10, 2020
1 parent 3852c18 commit d9aa1d4
Show file tree
Hide file tree
Showing 13 changed files with 35 additions and 22 deletions.
2 changes: 1 addition & 1 deletion ibm_cloud_sdk_core/api_exception.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ class ApiException(Exception):
global_transaction_id (str, optional): Globally unique id the service endpoint has given a transaction.
"""

def __init__(self, code: int, message: Optional[str] = None, http_response: Optional[Response] = None):
def __init__(self, code: int, *, message: Optional[str] = None, http_response: Optional[Response] = None):
# Call the base class constructor with the parameters it needs
super(ApiException, self).__init__(message)
self.message = message
Expand Down
4 changes: 3 additions & 1 deletion ibm_cloud_sdk_core/authenticators/cp4d_authenticator.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,11 +56,13 @@ def __init__(self,
username: str,
password: str,
url: str,
*,
disable_ssl_verification: bool = False,
headers: Optional[Dict[str, str]] = None,
proxies: Optional[Dict[str, str]] = None):
self.token_manager = CP4DTokenManager(
username, password, url, disable_ssl_verification, headers, proxies)
username, password, url, disable_ssl_verification=disable_ssl_verification,
headers=headers, proxies=proxies)
self.validate()

def validate(self):
Expand Down
6 changes: 4 additions & 2 deletions ibm_cloud_sdk_core/authenticators/iam_authenticator.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,15 +56,17 @@ class IAMAuthenticator(Authenticator):

def __init__(self,
apikey: str,
*,
url: Optional[str] = None,
client_id: Optional[str] = None,
client_secret: Optional[str] = None,
disable_ssl_verification: Optional[bool] = False,
headers: Optional[Dict[str, str]] = None,
proxies: Optional[Dict[str, str]] = None):
self.token_manager = IAMTokenManager(
apikey, url, client_id, client_secret, disable_ssl_verification,
headers, proxies)
apikey, url=url, client_id=client_id, client_secret=client_secret,
disable_ssl_verification=disable_ssl_verification,
headers=headers, proxies=proxies)
self.validate()

def validate(self):
Expand Down
6 changes: 4 additions & 2 deletions ibm_cloud_sdk_core/base_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ class BaseService:
'disable_ssl_verification option of the authenticator.'

def __init__(self,
*,
service_url: str = None,
authenticator: Authenticator = None,
disable_ssl_verification: bool = False):
Expand Down Expand Up @@ -218,8 +219,8 @@ def send(self, request: requests.Request, **kwargs) -> DetailedResponse:
result = response.json()
except:
result = response
return DetailedResponse(result, response.headers,
response.status_code)
return DetailedResponse(response=result, headers=response.headers,
status_code=response.status_code)

raise ApiException(
response.status_code, http_response=response)
Expand All @@ -237,6 +238,7 @@ def send(self, request: requests.Request, **kwargs) -> DetailedResponse:
def prepare_request(self,
method: str,
url: str,
*,
headers: Optional[dict] = None,
params: Optional[dict] = None,
data: Optional[Union[str, dict]] = None,
Expand Down
5 changes: 3 additions & 2 deletions ibm_cloud_sdk_core/cp4d_token_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ def __init__(self,
username: str,
password: str,
url: str,
*,
disable_ssl_verification: bool = False,
headers: Optional[Dict[str, str]] = None,
proxies: Optional[Dict[str, str]] = None):
Expand All @@ -61,8 +62,8 @@ def __init__(self,
url = url + '/v1/preauth/validateAuth'
self.headers = headers
self.proxies = proxies
super(CP4DTokenManager, self).__init__(url, disable_ssl_verification,
self.TOKEN_NAME)
super(CP4DTokenManager, self).__init__(url, disable_ssl_verification=disable_ssl_verification,
token_name=self.TOKEN_NAME)

def request_token(self) -> dict:
"""Makes a request for a token.
Expand Down
1 change: 1 addition & 0 deletions ibm_cloud_sdk_core/detailed_response.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ class DetailedResponse:
"""
def __init__(self,
*,
response: Optional[requests.Response] = None,
headers: Optional[Dict[str, str]] = None,
status_code: Optional[int] = None):
Expand Down
3 changes: 2 additions & 1 deletion ibm_cloud_sdk_core/iam_token_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ class IAMTokenManager(JWTTokenManager):

def __init__(self,
apikey: str,
*,
url: Optional[str] = None,
client_id: Optional[str] = None,
client_secret: Optional[str] = None,
Expand All @@ -76,7 +77,7 @@ def __init__(self,
self.headers = headers
self.proxies = proxies
super(IAMTokenManager, self).__init__(
self.url, disable_ssl_verification, self.TOKEN_NAME)
self.url, disable_ssl_verification=disable_ssl_verification, token_name=self.TOKEN_NAME)

def request_token(self) -> dict:
"""Request an IAM OAuth token given an API Key.
Expand Down
3 changes: 2 additions & 1 deletion ibm_cloud_sdk_core/jwt_token_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ class JWTTokenManager:

# pylint: disable=too-many-instance-attributes

def __init__(self, url: str, disable_ssl_verification: bool = False, token_name: Optional[str] = None):
def __init__(self, url: str, *, disable_ssl_verification: bool = False, token_name: Optional[str] = None):
self.url = url
self.disable_ssl_verification = disable_ssl_verification
self.token_name = token_name
Expand Down Expand Up @@ -200,6 +200,7 @@ def _save_token_info(self, token_response):
def _request(self,
method,
url,
*,
headers=None,
params=None,
data=None,
Expand Down
2 changes: 1 addition & 1 deletion ibm_cloud_sdk_core/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ def __read_from_env_variables(service_name: str) -> dict:
_parse_key_and_update_config(config, service_name, key, value)
return config

def __read_from_credential_file(service_name: str, separator: str = '=') -> dict:
def __read_from_credential_file(service_name: str, *, separator: str = '=') -> dict:
"""Return a config object based on credentials file for a service.
Args:
Expand Down
6 changes: 3 additions & 3 deletions test/test_base_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -524,7 +524,7 @@ def test_service_url_not_set():
assert str(err.value) == 'The service_url is required'

def test_setting_proxy():
service = BaseService('test', authenticator=IAMAuthenticator('wonder woman'))
service = BaseService(service_url='test', authenticator=IAMAuthenticator('wonder woman'))
assert service.authenticator is not None
assert service.authenticator.token_manager.http_config == {}

Expand All @@ -536,7 +536,7 @@ def test_setting_proxy():
service.set_http_config(http_config)
assert service.authenticator.token_manager.http_config == http_config

service2 = BaseService('test', authenticator=BasicAuthenticator('marvellous', 'mrs maisel'))
service2 = BaseService(service_url='test', authenticator=BasicAuthenticator('marvellous', 'mrs maisel'))
service2.set_http_config(http_config)
assert service2.authenticator is not None

Expand All @@ -551,7 +551,7 @@ def test_configure_service():
assert isinstance(service.get_authenticator(), NoAuthAuthenticator)

def test_configure_service_error():
service = BaseService('v1', authenticator=NoAuthAuthenticator())
service = BaseService(service_url='v1', authenticator=NoAuthAuthenticator())
with pytest.raises(ValueError) as err:
service.configure_service(None)
assert str(err.value) == 'Service_name must be of type string.'
6 changes: 4 additions & 2 deletions test/test_detailed_response.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ def test_detailed_response_dict():
content_type='application/json')

mock_response = requests.get('https://test.com')
detailed_response = DetailedResponse(mock_response.json(), mock_response.headers, mock_response.status_code)
detailed_response = DetailedResponse(response=mock_response.json(), headers=mock_response.headers,
status_code=mock_response.status_code)
assert detailed_response is not None
assert detailed_response.get_result() == {'foobar': 'baz'}
assert detailed_response.get_headers() == {u'Content-Type': 'application/json'}
Expand All @@ -39,7 +40,8 @@ def test_detailed_response_list():
content_type='application/json')

mock_response = requests.get('https://test.com')
detailed_response = DetailedResponse(mock_response.json(), mock_response.headers, mock_response.status_code)
detailed_response = DetailedResponse(response=mock_response.json(), headers=mock_response.headers,
status_code=mock_response.status_code)
assert detailed_response is not None
assert detailed_response.get_result() == ['foobar', 'baz']
assert detailed_response.get_headers() == {u'Content-Type': 'application/json'}
Expand Down
6 changes: 3 additions & 3 deletions test/test_iam_token_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ def test_request_token_auth_in_ctor():
default_auth_header = 'Basic Yng6Yng='
responses.add(responses.POST, url=iam_url, body=response, status=200)

token_manager = IAMTokenManager("apikey", iam_url, 'foo', 'bar')
token_manager = IAMTokenManager("apikey", url=iam_url, client_id='foo', client_secret='bar')
token_manager.request_token()

assert len(responses.calls) == 1
Expand Down Expand Up @@ -112,7 +112,7 @@ def test_request_token_auth_in_ctor_client_id_only():
}"""
responses.add(responses.POST, url=iam_url, body=response, status=200)

token_manager = IAMTokenManager("iam_apikey", iam_url, 'foo')
token_manager = IAMTokenManager("iam_apikey", url=iam_url, client_id='foo')
token_manager.request_token()

assert len(responses.calls) == 1
Expand All @@ -132,7 +132,7 @@ def test_request_token_auth_in_ctor_secret_only():
}"""
responses.add(responses.POST, url=iam_url, body=response, status=200)

token_manager = IAMTokenManager("iam_apikey", iam_url, None, 'bar')
token_manager = IAMTokenManager("iam_apikey", url=iam_url, client_id=None, client_secret='bar')
token_manager.request_token()

assert len(responses.calls) == 1
Expand Down
7 changes: 4 additions & 3 deletions test/test_jwt_token_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ def __init__(self, url=None, access_token=None):
self.url = url
self.access_token = access_token
self.request_count = 0 # just for tests to see how many times request was called
super(JWTTokenManagerMockImpl, self).__init__(url, access_token, 'access_token')
super(JWTTokenManagerMockImpl, self).__init__(url, disable_ssl_verification=access_token,
token_name='access_token')

def request_token(self):
self.request_count += 1
Expand Down Expand Up @@ -81,7 +82,7 @@ def test_paced_get_token():
assert token_manager.request_count == 1

def test_is_token_expired():
token_manager = JWTTokenManagerMockImpl(None, None)
token_manager = JWTTokenManagerMockImpl(None, access_token=None)
assert token_manager._is_token_expired() is True
token_manager.expire_time = _get_current_time() + 3600
assert token_manager._is_token_expired() is False
Expand All @@ -90,7 +91,7 @@ def test_is_token_expired():

def test_not_implemented_error():
with pytest.raises(NotImplementedError) as err:
token_manager = JWTTokenManager(None, None)
token_manager = JWTTokenManager(None, disable_ssl_verification=None)
token_manager.request_token()
assert str(err.value) == 'request_token MUST be overridden by a subclass of JWTTokenManager.'

Expand Down

0 comments on commit d9aa1d4

Please sign in to comment.