- Notifications
You must be signed in to change notification settings - Fork673
chore: makeGitlab.http_request() a private method#1842
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Changes fromall commits
File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading.Please reload this page.
Jump to
Uh oh!
There was an error while loading.Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -18,6 +18,7 @@ | ||
| import os | ||
| import time | ||
| import warnings | ||
| from typing import Any, cast, Dict, List, Optional, Tuple, TYPE_CHECKING, Union | ||
| import requests | ||
| @@ -549,7 +550,7 @@ def _build_url(self, path: str) -> str: | ||
| def _check_redirects(self, result: requests.Response) -> None: | ||
| # Check the requests history to detect 301/302 redirections. | ||
| # If the initialmethod is POST or PUT, the redirected request will use a | ||
| # GET request, leading to unwanted behaviour. | ||
| # If we detect a redirection with a POST or a PUT request, we | ||
| # raise an exception with a useful error message. | ||
| @@ -617,11 +618,45 @@ def http_request( | ||
| obey_rate_limit: bool = True, | ||
| max_retries: int = 10, | ||
| **kwargs: Any, | ||
| ) -> requests.Response: | ||
| warnings.warn( | ||
| "The Gitlab.http_request() method is deprecated and will be removed in a " | ||
| "future version. This is a private method and should not be used.", | ||
| DeprecationWarning, | ||
Comment on lines +622 to +625 Member There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. One thing about this method is that it provides a way for users to do stuff we haven't implemented (or perhaps even won't, if it's super specific). So I would maybe not be so strict about "should not be used". I would maybe say that API stability can't be guaranteed or something similar. Edit: I've now checked a bit, because I was thinking the same that we should hide a bit more of the interface. But when looking at the original author's initial implementation, I think this might have been a deliberate choice and the distinction was more of a higher vs. lower-level API. At least what I can see from the commit message and how it was done with other private methods that were not meant to be consumed, see:c5ad540.
So I've started working on some docs to cover use cases here:#1846 Some use cases I vaguely remember from issues include using HEAD to check file existence/size, getting headers from the server's response, and so on. I documented some of that. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. I agree with@nejch here. It's also useful for new functionality in the GitLab API. So let's tone the message down a bit. | ||
| ) | ||
| return self._http_request( | ||
| method=verb, | ||
| path=path, | ||
| query_data=query_data, | ||
| post_data=post_data, | ||
| raw=raw, | ||
| streamed=streamed, | ||
| files=files, | ||
| timeout=timeout, | ||
| obey_rate_limit=obey_rate_limit, | ||
| max_retries=max_retries, | ||
| **kwargs, | ||
| ) | ||
| def _http_request( | ||
| self, | ||
| *, | ||
| method: str, | ||
| path: str, | ||
| query_data: Optional[Dict[str, Any]] = None, | ||
| post_data: Optional[Union[Dict[str, Any], bytes]] = None, | ||
| raw: bool = False, | ||
| streamed: bool = False, | ||
| files: Optional[Dict[str, Any]] = None, | ||
| timeout: Optional[float] = None, | ||
| obey_rate_limit: bool = True, | ||
| max_retries: int = 10, | ||
| **kwargs: Any, | ||
| ) -> requests.Response: | ||
| """Make an HTTP request to the Gitlab server. | ||
| Args: | ||
| method: The HTTP method to call ('get', 'post', 'put', 'delete') | ||
| path: Path or full URL to query ('/projects' or | ||
| 'http://whatever/v4/api/projecs') | ||
| query_data: Data to send as query parameters | ||
| @@ -678,7 +713,7 @@ def http_request( | ||
| cur_retries = 0 | ||
| while True: | ||
| result = self.session.request( | ||
| method=method, | ||
| url=url, | ||
| json=json, | ||
| data=data, | ||
| @@ -758,8 +793,8 @@ def http_get( | ||
| GitlabParsingError: If the json data could not be parsed | ||
| """ | ||
| query_data = query_data or {} | ||
| result = self._http_request( | ||
| method="get",path=path, query_data=query_data, streamed=streamed, **kwargs | ||
| ) | ||
| if ( | ||
| @@ -855,9 +890,9 @@ def http_post( | ||
| query_data = query_data or {} | ||
| post_data = post_data or {} | ||
| result = self._http_request( | ||
| method="post", | ||
| path=path, | ||
| query_data=query_data, | ||
| post_data=post_data, | ||
| files=files, | ||
| @@ -903,9 +938,9 @@ def http_put( | ||
| query_data = query_data or {} | ||
| post_data = post_data or {} | ||
| result = self._http_request( | ||
| method="put", | ||
| path=path, | ||
| query_data=query_data, | ||
| post_data=post_data, | ||
| files=files, | ||
| @@ -933,7 +968,7 @@ def http_delete(self, path: str, **kwargs: Any) -> requests.Response: | ||
| Raises: | ||
| GitlabHttpError: When the return code is not 2xx | ||
| """ | ||
| return self._http_request(method="delete",path=path, **kwargs) | ||
| @gitlab.exceptions.on_http_error(gitlab.exceptions.GitlabSearchError) | ||
| def search( | ||
| @@ -987,7 +1022,9 @@ def _query( | ||
| self, url: str, query_data: Optional[Dict[str, Any]] = None, **kwargs: Any | ||
| ) -> None: | ||
| query_data = query_data or {} | ||
| result = self._gl._http_request( | ||
| method="get", path=url, query_data=query_data, **kwargs | ||
| ) | ||
| try: | ||
| links = result.links | ||
| if links: | ||