diff --git a/changelog.d/20241114_172127_chris_remove_tty_checks.rst b/changelog.d/20241114_172127_chris_remove_tty_checks.rst new file mode 100644 index 000000000..cce688919 --- /dev/null +++ b/changelog.d/20241114_172127_chris_remove_tty_checks.rst @@ -0,0 +1,12 @@ +Changed +^^^^^^^ + +- Bumped dependency on ``globus-sdk-python`` to at least `version 3.47.0 `_. + This version includes changes to detect ``EOFErrors`` when logging in with the command + line, obviating the need for existing ``globus-compute-sdk`` code that checks if a + user is in an interactive terminal before logging in. The old Compute code raised a + ``RuntimeError`` in that scenario; the new code raises a + ``globus_sdk.login_flows.CommandLineLoginFlowEOFError`` if Python's ``input`` + function raises an ``EOFError`` - which, in Compute, can happen if a previously + command-line-authenticated endpoint tries to re-authenticate but no longer has access + to a STDIN. diff --git a/compute_sdk/globus_compute_sdk/sdk/auth/globus_app.py b/compute_sdk/globus_compute_sdk/sdk/auth/globus_app.py index cd2c719dc..1c96e1e64 100644 --- a/compute_sdk/globus_compute_sdk/sdk/auth/globus_app.py +++ b/compute_sdk/globus_compute_sdk/sdk/auth/globus_app.py @@ -1,7 +1,6 @@ from __future__ import annotations import platform -import sys from globus_sdk import ClientApp, GlobusApp, GlobusAppConfig, UserApp @@ -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, @@ -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) diff --git a/compute_sdk/setup.py b/compute_sdk/setup.py index a1ccc34b4..edfcabf66 100644 --- a/compute_sdk/setup.py +++ b/compute_sdk/setup.py @@ -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.5.0", # 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 diff --git a/compute_sdk/tests/unit/auth/test_globus_app.py b/compute_sdk/tests/unit/auth/test_globus_app.py index a336ab29f..200c5c123 100644 --- a/compute_sdk/tests/unit/auth/test_globus_app.py +++ b/compute_sdk/tests/unit/auth/test_globus_app.py @@ -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." @@ -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() @@ -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) @@ -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)