Skip to content

Commit

Permalink
Remove code that checks if the user is in a TTY
Browse files Browse the repository at this point in the history
`globus-sdk==3.47.0` handles this class of errors for us by informing
users on what to do when `input` receives `EOFError` during login flows.
  • Loading branch information
chris-janidlo committed Nov 14, 2024
1 parent 55e0759 commit 6c26e7f
Show file tree
Hide file tree
Showing 3 changed files with 5 additions and 67 deletions.
31 changes: 3 additions & 28 deletions compute_sdk/globus_compute_sdk/sdk/auth/globus_app.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
from __future__ import annotations

import platform
import sys

from globus_sdk import ClientApp, GlobusApp, GlobusAppConfig, UserApp

Expand All @@ -11,21 +10,13 @@
DEFAULT_CLIENT_ID = "4cf29807-cf21-49ec-9443-ff9a3fb9f81c"


def _is_jupyter():
# Simplest way to find out if we are in Jupyter without having to
# check imports
return "jupyter_core" in sys.modules


def get_globus_app(environment: str | None = None):
def get_globus_app(environment: str | None = None) -> GlobusApp:
app_name = platform.node()
client_id, client_secret = get_client_creds()
config = GlobusAppConfig(token_storage=get_token_storage(environment=environment))

app: GlobusApp # silly mypy

if client_id and client_secret:
app = ClientApp(
return ClientApp(
app_name=app_name,
client_id=client_id,
client_secret=client_secret,
Expand All @@ -41,20 +32,4 @@ def get_globus_app(environment: str | None = None):

else:
client_id = client_id or DEFAULT_CLIENT_ID
app = UserApp(app_name=app_name, client_id=client_id, config=config)

# The authorization-via-web-link flow requires stdin; the user must visit
# the web link and enter generated code.
if (
app.login_required()
and (not sys.stdin.isatty() or sys.stdin.closed)
and not _is_jupyter()
):
# Not technically necessary; the login flow would just die with an EOF
# during input(), but adding this message here is much more direct --
# handle the non-happy path by letting the user know precisely the issue
raise RuntimeError(
"Unable to run native app login flow: stdin is closed or is not a TTY."
)

return app
return UserApp(app_name=app_name, client_id=client_id, config=config)
2 changes: 1 addition & 1 deletion compute_sdk/setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
REQUIRES = [
# request sending and authorization tools
"requests>=2.31.0,<3",
"globus-sdk>=3.46.0,<4",
"globus-sdk>=3.47.0,<4",
"globus-compute-common==0.4.1",
# dill is an extension of `pickle` to a wider array of native python types
# pin to the latest version, as 'dill' is not at 1.0 and does not have a clear
Expand Down
39 changes: 1 addition & 38 deletions compute_sdk/tests/unit/auth/test_globus_app.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

import pytest
from globus_compute_sdk.sdk.auth.globus_app import DEFAULT_CLIENT_ID, get_globus_app
from globus_sdk import ClientApp, GlobusApp, UserApp
from globus_sdk import ClientApp, UserApp
from pytest_mock import MockerFixture

_MOCK_BASE = "globus_compute_sdk.sdk.auth.globus_app."
Expand All @@ -17,9 +17,6 @@ def test_get_globus_app(
mocker.patch(
f"{_MOCK_BASE}get_client_creds", return_value=(client_id, client_secret)
)
mock_stdin = mocker.patch(f"{_MOCK_BASE}sys.stdin")
mock_stdin.isatty.return_value = True
mock_stdin.closed = False

app = get_globus_app()

Expand All @@ -37,9 +34,6 @@ def test_get_globus_app(
def test_get_globus_app_with_environment(mocker: MockerFixture, randomstring):
mock_get_token_storage = mocker.patch(f"{_MOCK_BASE}get_token_storage")
mocker.patch(f"{_MOCK_BASE}UserApp", autospec=True)
mock_stdin = mocker.patch(f"{_MOCK_BASE}sys.stdin")
mock_stdin.isatty.return_value = True
mock_stdin.closed = False

env = randomstring()
get_globus_app(environment=env)
Expand All @@ -52,34 +46,3 @@ def test_client_app_requires_creds(mocker: MockerFixture):
with pytest.raises(ValueError) as err:
get_globus_app()
assert "GLOBUS_COMPUTE_CLIENT_SECRET must be set" in str(err.value)


@pytest.mark.parametrize("login_required", [True, False])
@pytest.mark.parametrize("stdin_isatty", [True, False])
@pytest.mark.parametrize("stdin_closed", [True, False])
@pytest.mark.parametrize("is_jupyter", [True, False])
def test_user_app_login_requires_stdin(
mocker: MockerFixture,
login_required: bool,
stdin_isatty: bool,
stdin_closed: bool,
is_jupyter: bool,
):
mock_login_required = mocker.patch.object(GlobusApp, "login_required")
mock_is_jupyter = mocker.patch(f"{_MOCK_BASE}_is_jupyter")
mock_stdin = mocker.patch(f"{_MOCK_BASE}sys.stdin")

mock_login_required.return_value = login_required
mock_stdin.closed = stdin_closed
mock_stdin.isatty.return_value = stdin_isatty
mock_is_jupyter.return_value = is_jupyter

if login_required and (not stdin_isatty or stdin_closed) and not is_jupyter:
with pytest.raises(RuntimeError) as err:
get_globus_app()
assert "stdin is closed" in err.value.args[0]
assert "is not a TTY" in err.value.args[0]
assert "native app" in err.value.args[0]
else:
app = get_globus_app()
assert isinstance(app, UserApp)

0 comments on commit 6c26e7f

Please sign in to comment.