Conversation
albertsola
requested review from
alephsur,
d3rky,
jentyk,
robcsegal,
ruben-sebrango and
svazquezco
as code owners
albertsola
force-pushed
the
MPT-12360/async-HTTP-layer-with-Httpx
branch
from
17c6bbf to
6cc5002
Compare
albertsola
force-pushed
the
MPT-12360/async-HTTP-layer-with-Httpx
branch
2 times, most recently
from
fce7ab1 to
ddc956f
Compare
| class HTTPClient(Client): | ||
| """Sync HTTP client for interacting with SoftwareOne Marketplace Platform API.""" | ||
|
|
||
| def __init__( |
There was a problem hiding this comment.
This seems repeating in both classes so could be refactored to a Base Class. I think this is what you did in PR #18?
There was a problem hiding this comment.
PR #18 is about MPTClient, this one is about HTTPClient.
I would like to share that code, but that code is calling different parent depending on if it is a sync or an async client.
I do not know how to implement an init that calls a different parent depending on which "child" you are using, if you know how, please let me know.
There was a problem hiding this comment.
I think you did it elsewhere, just call super().__init__() with any params if needed of course
tests/http/conftest.py
Outdated
|
|
||
|
|
||
| @pytest.fixture | ||
| def mpt_client_async(api_url, api_token): |
There was a problem hiding this comment.
Is this a correct name? Shouldn't it be http_client_async? The same above?
| @@ -0,0 +1,57 @@ | |||
| import pytest | |||
There was a problem hiding this comment.
Just wondering. Maybe, to avoid repetition, since you seem copied the same tests for both clients, you could parametrize tests with sync/async clients?
There was a problem hiding this comment.
once are shared, but the important ones we need to use await in the http methods
albertsola
force-pushed
the
MPT-12360/async-HTTP-layer-with-Httpx
branch
from
ddc956f to
faad310
Compare
|
4 participants
No description provided.