Use the hostname and port for GraphQL connections #2878
Conversation
github/Requester.py
Outdated
| @@ -540,7 +540,7 @@ def graphql_query(self, query: str, variables: Dict[str, Any]) -> Tuple[Dict[str | |||
| """ | |||
| input_ = {"query": query, "variables": {"input": variables}} | |||
|
|
|||
| response_headers, data = self.requestJsonAndCheck("POST", "https://api.github.com/graphql", input=input_) | |||
| response_headers, data = self.requestJsonAndCheck("POST", f"https://{self.__hostname}:{self.__port}/graphql", input=input_) | |||
There was a problem hiding this comment.
Right, that is easier than I thought, but that should do it.
I'd suggest to construct the GraphQl URL in the __init__ method right after o = urllib.parse.urlparse(base_url) (line 397) like this:
o = urllib.parse.urlparse(base_url)
self.__graphql_url = f"{o.scheme}://{o.hostname}:{o.port}/graphql"
or maybe safer would be this
o = urllib.parse.urlparse(base_url)
self.__graphql_url = urlunparse(o._replace(path="graphql"))
And then use self.__graphql_url here instead:
| response_headers, data = self.requestJsonAndCheck("POST", f"https://{self.__hostname}:{self.__port}/graphql", input=input_) | |
| response_headers, data = self.requestJsonAndCheck("POST", self.__graphql_url, input=input_) |
There was a problem hiding this comment.
Above suggestion using urlunparse should also fix the test error ValueError: Port could not be cast to integer value as 'None'.
|
We could use some test here. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## main #2878 +/- ##
=======================================
Coverage 96.69% 96.69%
=======================================
Files 142 142
Lines 14533 14537 +4
=======================================
+ Hits 14053 14057 +4
Misses 480 480 ☔ View full report in Codecov by Sentry. |
|
I have fixed the imports... |
|
@EnricoMi Where's a good place I can find a test using enterprise GitHub? Thanks! |
|
I have added a few tests. Please test the code against your enterprise server. |
| @@ -52,6 +52,8 @@ | |||
| # Copyright 2023 Phillip Tran <phillip.qtr@gmail.com> # | |||
| # Copyright 2023 Trim21 <trim21.me@gmail.com> # | |||
| # Copyright 2023 adosibalo <94008816+adosibalo@users.noreply.github.com> # | |||
| # Copyright 2024 Enrico Minack <github@enrico.minack.dev> # | |||
| # Copyright 2024 nick <nick_shook@apple.com> # | |||
There was a problem hiding this comment.
I don't need attribution here, thanks!
|
This is close, our enterprise server is at |
|
What is the path of your v3 REST API? Is it |
|
yes, it's |
|
If you don't want the attribution, I have to do this without your commits: #2880 |
|
Fixed in #2880. |
Addresses #2870
@EnricoMi, I noticed that there are some ConnectionClasses, is that what you had envisioned? should the
requestJsonAndCheckmethod be moved to those classes?Thanks for this awesome lib!