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

Fix/user info #1079

Merged
merged 4 commits into from
Oct 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like def get_user_info doesn't take id as input. Can we handle that in this PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call, just added.

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,
)