Skip to content

Commit

Permalink
Fix/user info (#1079)
Browse files Browse the repository at this point in the history
Hopefully fixes
#1059

I think issue was that something on the server side wasn't happy with
using json body for get requests. This PR switches to including get
request query in the URL string.

Also updates the route to the correct `/user_info`
  • Loading branch information
cdbf1 authored Oct 2, 2024
1 parent 5efa23d commit 27d3910
Show file tree
Hide file tree
Showing 4 changed files with 60 additions and 20 deletions.
27 changes: 23 additions & 4 deletions general-superstaq/general_superstaq/service.py
Original file line number Diff line number Diff line change
Expand Up @@ -171,11 +171,29 @@ def get_user_info(self, *, name: str) -> list[dict[str, str | float]]: ...
@overload
def get_user_info(self, *, email: str) -> list[dict[str, str | float]]: ...

@overload
def get_user_info(self, *, user_id: int) -> list[dict[str, str | float]]: ...

@overload
def get_user_info(self, *, name: str, user_id: int) -> list[dict[str, str | float]]: ...

@overload
def get_user_info(self, *, email: str, user_id: int) -> list[dict[str, str | float]]: ...

@overload
def get_user_info(self, *, name: str, email: str) -> list[dict[str, str | float]]: ...

@overload
def get_user_info(
self, *, name: str | None = None, email: str | None = None
self, *, name: str, email: str, user_id: int
) -> list[dict[str, str | float]]: ...

def get_user_info(
self,
*,
name: str | None = None,
email: str | None = None,
user_id: int | None = None,
) -> dict[str, str | float] | list[dict[str, str | float]]:
"""Gets a dictionary of the user's info.
Expand All @@ -186,16 +204,17 @@ def get_user_info(
Args:
name: A name to search by. Defaults to None.
email: An email address to search by. Defaults to None
email: An email address to search by. Defaults to None.
user_id: A user ID to search by. Defaults to None.
Returns:
A dictionary of the user information. In the case that either the name or email
query kwarg is used, a list of dictionaries is returned, corresponding to the user
information for each user that matches the query.
"""
user_info = self._client.get_user_info(name=name, email=email)
user_info = self._client.get_user_info(name=name, email=email, user_id=user_id)

if name is None and email is None:
if name is None and email is None and user_id is None:
# If no query then return the only element in the list.
return user_info[0]

Expand Down
6 changes: 3 additions & 3 deletions general-superstaq/general_superstaq/service_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ def test_get_user_info(mock_get_request: mock.MagicMock) -> None:
"role": "free_trial",
"balance": 30.0,
}
mock_get_request.assert_called_once_with("/get_user_info", query={})
mock_get_request.assert_called_once_with("/user_info", query={})


@mock.patch(
Expand All @@ -120,7 +120,7 @@ def test_get_user_info_name_query(mock_get_request: mock.MagicMock) -> None:
"balance": 30.0,
}
]
mock_get_request.assert_called_once_with("/get_user_info", query={"name": "Alice"})
mock_get_request.assert_called_once_with("/user_info", query={"name": "Alice"})


@mock.patch(
Expand All @@ -145,7 +145,7 @@ def test_get_user_info_email_query(mock_get_request: mock.MagicMock) -> None:
"balance": 30.0,
}
]
mock_get_request.assert_called_once_with("/get_user_info", query={"email": "example@email.com"})
mock_get_request.assert_called_once_with("/user_info", query={"email": "example@email.com"})


@mock.patch(
Expand Down
19 changes: 13 additions & 6 deletions general-superstaq/general_superstaq/superstaq_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ def get_balance(self) -> dict[str, float]:
return self.get_request("/balance")

def get_user_info(
self, name: str | None = None, email: str | None = None
self, name: str | None = None, email: str | None = None, user_id: int | None = None
) -> list[dict[str, str | float]]:
"""Gets a dictionary of the user's info.
Expand All @@ -249,7 +249,8 @@ def get_user_info(
Args:
name: A name to search by. Defaults to None.
email: An email address to search by. Defaults to None
email: An email address to search by. Defaults to None.
user_id: A user ID to search by. Defaults to None.
Returns:
A list of dictionaries corresponding to the user
Expand All @@ -264,7 +265,9 @@ def get_user_info(
query["name"] = name
if email is not None:
query["email"] = email
user_info = self.get_request("/get_user_info", query=query)
if user_id is not None:
query["id"] = str(user_id)
user_info = self.get_request("/user_info", query=query)
if not user_info:
# Catch empty server response. This shouldn't happen as the server should return
# an error code if something is wrong with the request.
Expand Down Expand Up @@ -704,7 +707,8 @@ def get_request(self, endpoint: str, query: Mapping[str, object] | None = None)
Args:
endpoint: The endpoint to perform the GET request on.
query: An optional query json to include in the get request.
query: An optional query dictionary to include in the get request.
This query will be appended to the url.
Returns:
The response of the GET request.
Expand All @@ -716,11 +720,14 @@ def request() -> requests.Response:
Returns:
The Flask GET request object.
"""
if not query:
q_string = ""
else:
q_string = "?" + urllib.parse.urlencode(query)
return self.session.get(
f"{self.url}{endpoint}",
f"{self.url}{endpoint}{q_string}",
headers=self.headers,
verify=self.verify_https,
json=query,
)

response = self._make_request(request)
Expand Down
28 changes: 21 additions & 7 deletions general-superstaq/general_superstaq/superstaq_client_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -432,7 +432,6 @@ def test_superstaq_client_get_balance(mock_get: mock.MagicMock) -> None:
f"http://example.com/{API_VERSION}/balance",
headers=EXPECTED_HEADERS,
verify=False,
json=None,
)


Expand Down Expand Up @@ -990,9 +989,8 @@ def test_get_user_info(mock_get: mock.MagicMock) -> None:

user_info = client.get_user_info()
mock_get.assert_called_once_with(
f"http://example.com/{API_VERSION}/get_user_info",
f"http://example.com/{API_VERSION}/user_info",
headers=EXPECTED_HEADERS,
json={},
verify=False,
)
assert user_info == [{"Some": "Data"}]
Expand All @@ -1010,9 +1008,26 @@ def test_get_user_info_query(mock_get: mock.MagicMock) -> None:

user_info = client.get_user_info(name="Alice")
mock_get.assert_called_once_with(
f"http://example.com/{API_VERSION}/get_user_info",
f"http://example.com/{API_VERSION}/user_info?name=Alice",
headers=EXPECTED_HEADERS,
verify=False,
)
assert user_info == [{"Some": "Data"}]


@mock.patch("requests.Session.get")
def test_get_user_info_query_composite(mock_get: mock.MagicMock) -> None:
client = gss.superstaq_client._SuperstaqClient(
client_name="general-superstaq",
remote_host="http://example.com",
api_key="to_my_heart",
cq_token="cq-token",
)
mock_get.return_value.json.return_value = {"example@email.com": {"Some": "Data"}}
user_info = client.get_user_info(user_id=42, name="Alice")
mock_get.assert_called_once_with(
f"http://example.com/{API_VERSION}/user_info?name=Alice&id=42",
headers=EXPECTED_HEADERS,
json={"name": "Alice"},
verify=False,
)
assert user_info == [{"Some": "Data"}]
Expand All @@ -1035,8 +1050,7 @@ def test_get_user_info_empty_response(mock_get: mock.MagicMock) -> None:
client.get_user_info()

mock_get.assert_called_once_with(
f"http://example.com/{API_VERSION}/get_user_info",
f"http://example.com/{API_VERSION}/user_info",
headers=EXPECTED_HEADERS,
json={},
verify=False,
)

0 comments on commit 27d3910

Please sign in to comment.