Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace usages of Client.get_object by Client.request #383

Closed
browniebroke opened this issue Oct 9, 2021 · 2 comments · Fixed by #394
Closed

Replace usages of Client.get_object by Client.request #383

browniebroke opened this issue Oct 9, 2021 · 2 comments · Fixed by #394
Labels
help wanted Extra attention is needed

Comments

@browniebroke
Copy link
Owner

Most of the client methods use the get_object method internally:

def get_object(
self, object_t, object_id=None, relation=None, parent=None, **params
):
"""
Actually query the Deezer API to retrieve the object.
:returns: an :class:`~deezer.resources.Resource` or subclass.
"""
url = self.object_url(object_t, object_id, relation)
params = params or {}
if self.access_token is not None:
params["access_token"] = str(self.access_token)
response = self.session.get(url, params=params)
json_data = response.json()
if "error" in json_data:
raise ValueError(
f"API request return error for object: {object_t} id: {object_id}"
)
return self._process_json(json_data, parent)

In #379, the request method was added:

def request(self, method: str, path: str, **params):
"""
Make a request to the API and parse the response.
:param method: HTTP verb to use: GET, POST< DELETE, ...
:param path: The path to make the API call to (e.g. 'artist/1234')
:param params: Query parameters to add the the request
:return:
"""
if self.access_token is not None:
params["access_token"] = str(self.access_token)
response = self.session.request(
method,
f"{self.base_url}/{path}",
params=params,
)
try:
response.raise_for_status()
except requests.HTTPError as exc:
raise DeezerHTTPError.from_http_error(exc) from exc
json_data = response.json()
if not isinstance(json_data, dict):
return json_data
if "error" in json_data:
raise DeezerErrorResponse(json_data)
return self._process_json(json_data)

They are very similar, but the request method is more versatile, we should aim to migrate all usages to the latter and eventually delete the get_object method.

@browniebroke browniebroke added good first issue Good for newcomers hacktoberfest Good issues for Hacktoberfest labels Oct 9, 2021
@deepakdinesh1123
Copy link

Hi, can I take this issue?

@browniebroke
Copy link
Owner Author

browniebroke commented Oct 10, 2021

Feel free to take it, yes. It might involve a good amount of repetitive changes.

Also, I realised that the 2 methods raise a slightly different exception in case of error so this would be a breaking change that will require a major version bump. We need to mark this change as such as explained in the conventioanl commits spec.

@browniebroke browniebroke added help wanted Extra attention is needed and removed good first issue Good for newcomers hacktoberfest Good issues for Hacktoberfest labels Oct 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants