Skip to content

Able to resolve propery HTTP client when http option is str#1

Closed
encyphered wants to merge 1 commit intoAccelByte:mainfrom
encyphered:feature/fix-initialization-bug-when-str-http-option
Closed

Able to resolve propery HTTP client when http option is str#1
encyphered wants to merge 1 commit intoAccelByte:mainfrom
encyphered:feature/fix-initialization-bug-when-str-http-option

Conversation

@encyphered
Copy link
Copy Markdown

Hi, I'm Geunwoo from the Krafton GPP team.

https://github.com/AccelByte/accelbyte-python-sdk/blob/09c1e98/accelbyte_py_sdk/core/_core.py#L83-L90

        http_client = options["http"]
        if isinstance(http_client, str):
            implementation = next((impl for impl in _HTTP_CLIENT_IMPL if impl.__name__ == http_client), None)
            if implementation is None:
                raise ValueError(f"HTTP Client '{token_repository}' not recognized.")
        elif not isinstance(http_client, HttpClient):
            raise ValueError(f"HTTP Client '{token_repository}' not recognized.")
        _HTTP_CLIENT = http_client

In initialization function, accelbyte_py_sdk.core.initialize, we can define HTTP client to use by passing options with 'http' dictionary key. The value could be str or class but when the type of the value is str, it can resolve implementation from _HTTP_CLIENT_IMPL but object initialization is lack. It assigns just string value to the global _HTTP_CLIENT variable and it makes an error. Fixing this.

Reproducing:

    def test_token_grant_v3(self):
        initialize(options={
            'http': 'RequestsHttpClient',
        })
        res, err = token_grant_v3('client_credentials')
        self.assertIsNone(err)

will occur

Traceback (most recent call last):
  File "/Users/cypher/.pyenv/versions/3.7.6/lib/python3.7/unittest/case.py", line 59, in testPartExecutor
    yield
  File "/Users/cypher/.pyenv/versions/3.7.6/lib/python3.7/unittest/case.py", line 628, in run
    testMethod()
  File "/Users/cypher/Projects/accelbyte-python-sdk/test/unit/test_http_client.py", line 24, in test_token_grant_v3
    res, err = token_grant_v3('client_credentials')
  File "/Users/cypher/Projects/accelbyte-python-sdk/accelbyte_py_sdk/api/iam/wrappers/_o_auth2_0.py", line 130, in token_grant_v3
    return run_request(request)
  File "/Users/cypher/Projects/accelbyte-python-sdk/accelbyte_py_sdk/core/_core.py", line 254, in run_request
    request, error = http_client.create_request(operation, base_url, headers, **kwargs)
AttributeError: 'str' object has no attribute 'create_request'

@dhanarab
Copy link
Copy Markdown
Contributor

Hi, our engineer have taken a look on this PR and it looks good. It is accepted.

However, currently our repository setup does not allow to merge this PR directly on GitHub. So, our engineer have taken this PR and merge it into our internal repo before releasing it on GitHub. It is included in this release https://github.com/AccelByte/accelbyte-python-sdk/releases/tag/v0.2.1.

Tell us if you have additional input on this.

Thank you.

@dhanarab dhanarab closed this Oct 13, 2021
@encyphered encyphered deleted the feature/fix-initialization-bug-when-str-http-option branch October 13, 2021 17:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants