From 58efc88144b7ff0bd0d35d601e821da1d4a6f23e Mon Sep 17 00:00:00 2001 From: Matt Fisher Date: Mon, 10 Feb 2025 09:01:33 -0700 Subject: [PATCH 01/25] Allow user to control behavior when login fails; warn by default Better distinguish between a failed login attempt and a skipped strategy --- earthaccess/api.py | 37 +++++++++++++--- earthaccess/auth.py | 89 +++++++++++++++++++++++++++------------ earthaccess/exceptions.py | 13 ++++++ 3 files changed, 107 insertions(+), 32 deletions(-) create mode 100644 earthaccess/exceptions.py diff --git a/earthaccess/api.py b/earthaccess/api.py index b6ba1a38..5a067910 100644 --- a/earthaccess/api.py +++ b/earthaccess/api.py @@ -4,9 +4,19 @@ import requests import s3fs from fsspec import AbstractFileSystem -from typing_extensions import Any, Dict, List, Mapping, Optional, Union, deprecated +from typing_extensions import ( + Any, + Dict, + List, + Literal, + Mapping, + Optional, + Union, + deprecated, +) import earthaccess +from earthaccess.exceptions import LoginStrategyUnavailable from earthaccess.services import DataServices from .auth import Auth @@ -161,7 +171,12 @@ def search_services(count: int = -1, **kwargs: Any) -> List[Any]: return query.get(hits if count < 1 else min(count, hits)) -def login(strategy: str = "all", persist: bool = False, system: System = PROD) -> Auth: +def login( + strategy: str = "all", + persist: bool = False, + system: System = PROD, + on_failure: Literal["warn", "error"] = "warn", +) -> Auth: """Authenticate with Earthdata login (https://urs.earthdata.nasa.gov/). Parameters: @@ -174,6 +189,7 @@ def login(strategy: str = "all", persist: bool = False, system: System = PROD) - * **"environment"**: retrieve username and password from `$EARTHDATA_USERNAME` and `$EARTHDATA_PASSWORD`. persist: will persist credentials in a .netrc file system: the Earthdata system to access, defaults to PROD + on_failure: Whether to print a warning or raise an error when login fails. Returns: An instance of Auth. @@ -186,16 +202,25 @@ def login(strategy: str = "all", persist: bool = False, system: System = PROD) - for strategy in ["environment", "netrc", "interactive"]: try: earthaccess.__auth__.login( - strategy=strategy, persist=persist, system=system + strategy=strategy, + persist=persist, + system=system, + on_failure=on_failure, ) - except Exception: - pass + except LoginStrategyUnavailable as err: + logger.debug(err) + continue if earthaccess.__auth__.authenticated: earthaccess.__store__ = Store(earthaccess.__auth__) break else: - earthaccess.__auth__.login(strategy=strategy, persist=persist, system=system) + earthaccess.__auth__.login( + strategy=strategy, + persist=persist, + system=system, + on_failure=on_failure, + ) if earthaccess.__auth__.authenticated: earthaccess.__store__ = Store(earthaccess.__auth__) diff --git a/earthaccess/auth.py b/earthaccess/auth.py index 3a3b209c..9df4e795 100644 --- a/earthaccess/auth.py +++ b/earthaccess/auth.py @@ -6,15 +6,16 @@ import shutil from netrc import NetrcParseError from pathlib import Path -from typing import Any, Dict, List, Mapping, Optional +from typing import Any, Dict, List, Literal, Mapping, Optional from urllib.parse import urlparse import requests # type: ignore from tinynetrc import Netrc from typing_extensions import deprecated -from .daac import DAACS -from .system import PROD, System +from earthaccess.daac import DAACS +from earthaccess.exceptions import LoginAttemptFailure, LoginStrategyUnavailable +from earthaccess.system import PROD, System try: user_agent = f"earthaccess v{importlib.metadata.version('earthaccess')}" @@ -99,6 +100,8 @@ def login( strategy: str = "netrc", persist: bool = False, system: Optional[System] = None, + *, + on_failure: Literal["warn", "error"], ) -> Any: """Authenticate with Earthdata login. @@ -112,6 +115,7 @@ def login( Retrieve a username and password from $EARTHDATA_USERNAME and $EARTHDATA_PASSWORD. persist: Will persist credentials in a `.netrc` file. system: the EDL endpoint to log in to Earthdata, defaults to PROD + on_failure: Whether to print a warning or raise an error when login fails. Returns: An instance of Auth. @@ -124,11 +128,11 @@ def login( return self if strategy == "interactive": - self._interactive(persist) + self._interactive(persist, on_failure=on_failure) elif strategy == "netrc": - self._netrc() + self._netrc(on_failure=on_failure) elif strategy == "environment": - self._environment() + self._environment(on_failure=on_failure) return self @@ -234,63 +238,96 @@ class Session instance with Auth and bearer token headers ) return session - def _interactive(self, persist_credentials: bool = False) -> bool: + def _interactive( + self, + persist_credentials: bool = False, + *, + on_failure: Literal["warn", "error"] = "warn", + ) -> bool: username = input("Enter your Earthdata Login username: ") password = getpass.getpass(prompt="Enter your Earthdata password: ") - authenticated = self._get_credentials(username, password) + authenticated = self._get_credentials(username, password, on_failure=on_failure) if authenticated: logger.debug("Using user provided credentials for EDL") if persist_credentials: self._persist_user_credentials(username, password) return authenticated - def _netrc(self) -> bool: + def _netrc( + self, + *, + on_failure: Literal["warn", "error"], + ) -> bool: netrc_loc = netrc_path() try: my_netrc = Netrc(str(netrc_loc)) except FileNotFoundError as err: - raise FileNotFoundError(f"No .netrc found at {netrc_loc}") from err + raise LoginStrategyUnavailable(f"No .netrc found at {netrc_loc}") from err except NetrcParseError as err: - raise NetrcParseError(f"Unable to parse .netrc file {netrc_loc}") from err + raise LoginStrategyUnavailable( + f"Unable to parse .netrc file {netrc_loc}" + ) from err - if (creds := my_netrc[self.system.edl_hostname]) is None: + creds = my_netrc[self.system.edl_hostname] + if creds is None: return False username = creds["login"] password = creds["password"] - authenticated = self._get_credentials(username, password) + + if username is None: + raise LoginStrategyUnavailable( + f"Username not found in .netrc file {netrc_loc}" + ) + if password is None: + raise LoginStrategyUnavailable( + f"Password not found in .netrc file {netrc_loc}" + ) + + authenticated = self._get_credentials(username, password, on_failure=on_failure) if authenticated: logger.debug("Using .netrc file for EDL") return authenticated - def _environment(self) -> bool: + def _environment( + self, + *, + on_failure: Literal["warn", "error"], + ) -> bool: username = os.getenv("EARTHDATA_USERNAME") password = os.getenv("EARTHDATA_PASSWORD") - authenticated = self._get_credentials(username, password) - if authenticated: - logger.debug("Using environment variables for EDL") - else: - logger.debug( + + if not username or not password: + raise LoginStrategyUnavailable( "EARTHDATA_USERNAME and EARTHDATA_PASSWORD are not set in the current environment, try " "setting them or use a different strategy (netrc, interactive)" ) - return authenticated + + logger.debug("Using environment variables for EDL") + return self._get_credentials(username, password, on_failure=on_failure) def _get_credentials( - self, username: Optional[str], password: Optional[str] + self, + username: Optional[str], + password: Optional[str], + *, + on_failure: Literal["warn", "error"], ) -> bool: if username is not None and password is not None: token_resp = self._find_or_create_token(username, password) if not (token_resp.ok): # type: ignore - logger.info( - f"Authentication with Earthdata Login failed with:\n{token_resp.text}" - ) - return False - logger.debug("You're now authenticated with NASA Earthdata Login") + msg = f"Authentication with Earthdata Login failed with:\n{token_resp.text}" + if on_failure == "warn": + logger.warning(msg) + return False + elif on_failure == "error": + raise LoginAttemptFailure(msg) + + logger.info("You're now authenticated with NASA Earthdata Login") self.username = username self.password = password diff --git a/earthaccess/exceptions.py b/earthaccess/exceptions.py new file mode 100644 index 00000000..452a1a26 --- /dev/null +++ b/earthaccess/exceptions.py @@ -0,0 +1,13 @@ +class LoginStrategyUnavailable(Exception): + """The selected login strategy was skipped. + + This should be raised when a login strategy can't be attempted, for example because + "environment" was selected but the envvars are not populated. + + DO NOT raise this exception when a login strategy is attempted and failed, for + example because credentials were rejected. + """ + + +class LoginAttemptFailure(Exception): + """The login attempt failed.""" From 2571e10bf6d398e257ce4fe9641f8050b8e357fb Mon Sep 17 00:00:00 2001 From: Matt Fisher Date: Mon, 10 Feb 2025 09:25:14 -0700 Subject: [PATCH 02/25] Extract type alias --- earthaccess/api.py | 5 ++--- earthaccess/auth.py | 11 ++++++----- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/earthaccess/api.py b/earthaccess/api.py index 5a067910..93f2f595 100644 --- a/earthaccess/api.py +++ b/earthaccess/api.py @@ -8,7 +8,6 @@ Any, Dict, List, - Literal, Mapping, Optional, Union, @@ -19,7 +18,7 @@ from earthaccess.exceptions import LoginStrategyUnavailable from earthaccess.services import DataServices -from .auth import Auth +from .auth import Auth, LoginFailureBehavior from .results import DataCollection, DataGranule from .search import CollectionQuery, DataCollections, DataGranules, GranuleQuery from .store import Store @@ -175,7 +174,7 @@ def login( strategy: str = "all", persist: bool = False, system: System = PROD, - on_failure: Literal["warn", "error"] = "warn", + on_failure: LoginFailureBehavior = "warn", ) -> Auth: """Authenticate with Earthdata login (https://urs.earthdata.nasa.gov/). diff --git a/earthaccess/auth.py b/earthaccess/auth.py index 9df4e795..cee0037d 100644 --- a/earthaccess/auth.py +++ b/earthaccess/auth.py @@ -24,6 +24,7 @@ logger = logging.getLogger(__name__) +LoginFailureBehavior = Literal["error", "warn"] def netrc_path() -> Path: @@ -101,7 +102,7 @@ def login( persist: bool = False, system: Optional[System] = None, *, - on_failure: Literal["warn", "error"], + on_failure: LoginFailureBehavior, ) -> Any: """Authenticate with Earthdata login. @@ -242,7 +243,7 @@ def _interactive( self, persist_credentials: bool = False, *, - on_failure: Literal["warn", "error"] = "warn", + on_failure: LoginFailureBehavior = "warn", ) -> bool: username = input("Enter your Earthdata Login username: ") password = getpass.getpass(prompt="Enter your Earthdata password: ") @@ -256,7 +257,7 @@ def _interactive( def _netrc( self, *, - on_failure: Literal["warn", "error"], + on_failure: LoginFailureBehavior, ) -> bool: netrc_loc = netrc_path() @@ -295,7 +296,7 @@ def _netrc( def _environment( self, *, - on_failure: Literal["warn", "error"], + on_failure: LoginFailureBehavior, ) -> bool: username = os.getenv("EARTHDATA_USERNAME") password = os.getenv("EARTHDATA_PASSWORD") @@ -314,7 +315,7 @@ def _get_credentials( username: Optional[str], password: Optional[str], *, - on_failure: Literal["warn", "error"], + on_failure: LoginFailureBehavior, ) -> bool: if username is not None and password is not None: token_resp = self._find_or_create_token(username, password) From 775976e339df10e9cd3477ccfd2de02e7cc3d5e6 Mon Sep 17 00:00:00 2001 From: Matt Fisher Date: Mon, 10 Feb 2025 09:27:15 -0700 Subject: [PATCH 03/25] Give the top-level strategy function a default for "on_failure" --- earthaccess/auth.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/earthaccess/auth.py b/earthaccess/auth.py index cee0037d..43d1bfa2 100644 --- a/earthaccess/auth.py +++ b/earthaccess/auth.py @@ -102,7 +102,7 @@ def login( persist: bool = False, system: Optional[System] = None, *, - on_failure: LoginFailureBehavior, + on_failure: LoginFailureBehavior = "warn", ) -> Any: """Authenticate with Earthdata login. From a8b204c41bb5cb49e82ad822b27bacb56f312b24 Mon Sep 17 00:00:00 2001 From: Matt Fisher Date: Mon, 10 Feb 2025 09:36:26 -0700 Subject: [PATCH 04/25] Fix integration test --- tests/integration/test_auth.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/integration/test_auth.py b/tests/integration/test_auth.py index 1a83833d..112ef3b7 100644 --- a/tests/integration/test_auth.py +++ b/tests/integration/test_auth.py @@ -5,6 +5,7 @@ import pytest import requests import s3fs +from earthaccess.exceptions import LoginStrategyUnavailable logger = logging.getLogger(__name__) @@ -24,8 +25,8 @@ def test_auth_can_read_from_netrc_file(mock_netrc): assert auth.authenticated -def test_auth_throws_exception_if_netrc_is_not_present(mock_missing_netrc): - with pytest.raises(FileNotFoundError): +def test_auth_strategy_unavailable_netrc_is_not_present(mock_missing_netrc): + with pytest.raises(LoginStrategyUnavailable): earthaccess.login(strategy="netrc") From f8e74b49024bc2046fbbd3ea2e87e4731144fe85 Mon Sep 17 00:00:00 2001 From: Matt Fisher Date: Mon, 10 Feb 2025 09:39:34 -0700 Subject: [PATCH 05/25] Make on_failure kw-only --- earthaccess/api.py | 1 + 1 file changed, 1 insertion(+) diff --git a/earthaccess/api.py b/earthaccess/api.py index 93f2f595..7f2c8ef4 100644 --- a/earthaccess/api.py +++ b/earthaccess/api.py @@ -174,6 +174,7 @@ def login( strategy: str = "all", persist: bool = False, system: System = PROD, + *, on_failure: LoginFailureBehavior = "warn", ) -> Auth: """Authenticate with Earthdata login (https://urs.earthdata.nasa.gov/). From b2787428ef2aaab225dcf9246ad867ece59f9a3a Mon Sep 17 00:00:00 2001 From: Matt Fisher Date: Mon, 10 Feb 2025 10:55:09 -0700 Subject: [PATCH 06/25] Change argument to boolean, default to error --- earthaccess/api.py | 10 +++++----- earthaccess/auth.py | 46 ++++++++++++++++++++++++++++----------------- 2 files changed, 34 insertions(+), 22 deletions(-) diff --git a/earthaccess/api.py b/earthaccess/api.py index 7f2c8ef4..113ca7ba 100644 --- a/earthaccess/api.py +++ b/earthaccess/api.py @@ -18,7 +18,7 @@ from earthaccess.exceptions import LoginStrategyUnavailable from earthaccess.services import DataServices -from .auth import Auth, LoginFailureBehavior +from .auth import Auth from .results import DataCollection, DataGranule from .search import CollectionQuery, DataCollections, DataGranules, GranuleQuery from .store import Store @@ -175,7 +175,7 @@ def login( persist: bool = False, system: System = PROD, *, - on_failure: LoginFailureBehavior = "warn", + raise_on_failure: bool = True, ) -> Auth: """Authenticate with Earthdata login (https://urs.earthdata.nasa.gov/). @@ -189,7 +189,7 @@ def login( * **"environment"**: retrieve username and password from `$EARTHDATA_USERNAME` and `$EARTHDATA_PASSWORD`. persist: will persist credentials in a .netrc file system: the Earthdata system to access, defaults to PROD - on_failure: Whether to print a warning or raise an error when login fails. + raise_on_failure: Whether to raise an error when login fails; if false, a warning will be printed Returns: An instance of Auth. @@ -205,7 +205,7 @@ def login( strategy=strategy, persist=persist, system=system, - on_failure=on_failure, + raise_on_failure=raise_on_failure, ) except LoginStrategyUnavailable as err: logger.debug(err) @@ -219,7 +219,7 @@ def login( strategy=strategy, persist=persist, system=system, - on_failure=on_failure, + raise_on_failure=raise_on_failure, ) if earthaccess.__auth__.authenticated: earthaccess.__store__ = Store(earthaccess.__auth__) diff --git a/earthaccess/auth.py b/earthaccess/auth.py index 43d1bfa2..611021af 100644 --- a/earthaccess/auth.py +++ b/earthaccess/auth.py @@ -6,7 +6,7 @@ import shutil from netrc import NetrcParseError from pathlib import Path -from typing import Any, Dict, List, Literal, Mapping, Optional +from typing import Any, Dict, List, Mapping, Optional from urllib.parse import urlparse import requests # type: ignore @@ -24,7 +24,6 @@ logger = logging.getLogger(__name__) -LoginFailureBehavior = Literal["error", "warn"] def netrc_path() -> Path: @@ -102,7 +101,7 @@ def login( persist: bool = False, system: Optional[System] = None, *, - on_failure: LoginFailureBehavior = "warn", + raise_on_failure: bool = True, ) -> Any: """Authenticate with Earthdata login. @@ -116,7 +115,7 @@ def login( Retrieve a username and password from $EARTHDATA_USERNAME and $EARTHDATA_PASSWORD. persist: Will persist credentials in a `.netrc` file. system: the EDL endpoint to log in to Earthdata, defaults to PROD - on_failure: Whether to print a warning or raise an error when login fails. + raise_on_failure: Whether to raise an error when login fails; if false, a warning will be printed Returns: An instance of Auth. @@ -129,11 +128,11 @@ def login( return self if strategy == "interactive": - self._interactive(persist, on_failure=on_failure) + self._interactive(persist, raise_on_failure=raise_on_failure) elif strategy == "netrc": - self._netrc(on_failure=on_failure) + self._netrc(raise_on_failure=raise_on_failure) elif strategy == "environment": - self._environment(on_failure=on_failure) + self._environment(raise_on_failure=raise_on_failure) return self @@ -243,11 +242,15 @@ def _interactive( self, persist_credentials: bool = False, *, - on_failure: LoginFailureBehavior = "warn", + raise_on_failure: bool, ) -> bool: username = input("Enter your Earthdata Login username: ") password = getpass.getpass(prompt="Enter your Earthdata password: ") - authenticated = self._get_credentials(username, password, on_failure=on_failure) + authenticated = self._get_credentials( + username, + password, + raise_on_failure=raise_on_failure, + ) if authenticated: logger.debug("Using user provided credentials for EDL") if persist_credentials: @@ -257,7 +260,7 @@ def _interactive( def _netrc( self, *, - on_failure: LoginFailureBehavior, + raise_on_failure: bool, ) -> bool: netrc_loc = netrc_path() @@ -286,7 +289,11 @@ def _netrc( f"Password not found in .netrc file {netrc_loc}" ) - authenticated = self._get_credentials(username, password, on_failure=on_failure) + authenticated = self._get_credentials( + username, + password, + raise_on_failure=raise_on_failure, + ) if authenticated: logger.debug("Using .netrc file for EDL") @@ -296,7 +303,7 @@ def _netrc( def _environment( self, *, - on_failure: LoginFailureBehavior, + raise_on_failure: bool, ) -> bool: username = os.getenv("EARTHDATA_USERNAME") password = os.getenv("EARTHDATA_PASSWORD") @@ -308,25 +315,30 @@ def _environment( ) logger.debug("Using environment variables for EDL") - return self._get_credentials(username, password, on_failure=on_failure) + return self._get_credentials( + username, + password, + raise_on_failure=raise_on_failure, + ) def _get_credentials( self, username: Optional[str], password: Optional[str], *, - on_failure: LoginFailureBehavior, + raise_on_failure: bool, ) -> bool: if username is not None and password is not None: token_resp = self._find_or_create_token(username, password) if not (token_resp.ok): # type: ignore msg = f"Authentication with Earthdata Login failed with:\n{token_resp.text}" - if on_failure == "warn": + if raise_on_failure: + logger.error(msg) + raise LoginAttemptFailure(msg) + else: logger.warning(msg) return False - elif on_failure == "error": - raise LoginAttemptFailure(msg) logger.info("You're now authenticated with NASA Earthdata Login") self.username = username From 77d75a8620084502b425adde6078e2189b3d755f Mon Sep 17 00:00:00 2001 From: Matt Fisher Date: Mon, 10 Feb 2025 11:06:41 -0700 Subject: [PATCH 07/25] Fix unit tests --- tests/unit/test_auth.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/tests/unit/test_auth.py b/tests/unit/test_auth.py index 2f93b9ce..071e2a52 100644 --- a/tests/unit/test_auth.py +++ b/tests/unit/test_auth.py @@ -6,6 +6,7 @@ import pytest import responses from earthaccess import Auth +from earthaccess.exceptions import LoginAttemptFailure logger = logging.getLogger(__name__) @@ -95,9 +96,7 @@ def test_auth_fails_for_wrong_credentials(self, user_input, user_password): ) # Test auth = Auth() - auth.login(strategy="interactive") - with pytest.raises(Exception) as e_info: - self.assertEqual(auth.authenticated, False) - self.assertEqual(e_info, Exception) - self.assertEqual(auth.password, "password") - self.assertEqual(auth.token, json_response) + with pytest.raises(LoginAttemptFailure): + auth.login(strategy="interactive") + + self.assertEqual(auth.authenticated, False) From 6f2cabf944372616268d7e8eda66a6734126ec85 Mon Sep 17 00:00:00 2001 From: Matt Fisher Date: Mon, 10 Feb 2025 11:10:13 -0700 Subject: [PATCH 08/25] Add breaking change to changelog For some reason the final change in this changeset was causing my editor to fail to parse markdown after that point, so I adjusted the line length and that fixed it. --- CHANGELOG.md | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3f393dd4..38e146eb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,18 +7,25 @@ and this project uses [Semantic Versioning](https://semver.org/spec/v2.0.0.html) ## [Unreleased] -### Changed +### Added - `search_datasets` now accepts a `has_granules` keyword argument. Use `has_granules=False` to search for metadata about collections with no associated granules. The default value set in `DataCollections` remains `True`. ([#939](https://github.com/nsidc/earthaccess/issues/939)) ([**@juliacollins**](https://github.com/juliacollins)) +### Changed +- **Breaking**: Throw an error by default when login credentials are rejected. Pass + `earthaccess.login(..., raise_on_failure=False)` to disable and print a warning + instead. ([#946](https://github.com/nsidc/earthaccess/pull/946)) + ([**@mfisher87**](https://github.com/mfisher87)) + ## [v0.13.0] - 2025-01-28 ### Changed -- Integration tests: Test are no longer randomized! this means each fail should be reproducible, we are testing the most - popular datasets from all DAACs, see files under tests/integration/popular_collections. +- Integration tests: Test are no longer randomized! this means each fail should be + reproducible, we are testing the most popular datasets from all DAACs, see files under + tests/integration/popular_collections. ([#215](https://github.com/nsidc/earthaccess/issues/215)) ([**@mfisher87**](https://github.com/mfisher87)) From 763fcbe042469e2c3aa8b1a8e49b5f5dfb18e6a3 Mon Sep 17 00:00:00 2001 From: Matt Fisher Date: Mon, 10 Feb 2025 11:13:32 -0700 Subject: [PATCH 09/25] Add migration guide for next release --- docs/user_guide/backwards-compatibility.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/docs/user_guide/backwards-compatibility.md b/docs/user_guide/backwards-compatibility.md index d11636dd..2c4e060b 100644 --- a/docs/user_guide/backwards-compatibility.md +++ b/docs/user_guide/backwards-compatibility.md @@ -43,6 +43,8 @@ Our project follows the [SPEC0](https://scientific-python.org/specs/spec-0000/) ## Migration guides -Under Development 🚧 +### 0.14.0 -This section will contain migration guides for future releases. Check back soon for updates! +* `earthaccess.login()` will now fail by default if Earthdata Login rejects credentials. + We recommend you adjust your code to handle this. If you'd really like to disable + this, pass `earthaccess.login(..., raise_on_failure=False)`. From ce1b44c9992b78b29506548635a94c9b7c7bb120 Mon Sep 17 00:00:00 2001 From: Matt Fisher <3608264+mfisher87@users.noreply.github.com> Date: Mon, 10 Feb 2025 15:58:46 -0700 Subject: [PATCH 10/25] Clarify migration language Co-authored-by: Chuck Daniels --- docs/user_guide/backwards-compatibility.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/user_guide/backwards-compatibility.md b/docs/user_guide/backwards-compatibility.md index 2c4e060b..fa40ab03 100644 --- a/docs/user_guide/backwards-compatibility.md +++ b/docs/user_guide/backwards-compatibility.md @@ -45,6 +45,6 @@ Our project follows the [SPEC0](https://scientific-python.org/specs/spec-0000/) ### 0.14.0 -* `earthaccess.login()` will now fail by default if Earthdata Login rejects credentials. +* `earthaccess.login()` will now raise an exception by default if Earthdata Login rejects credentials. We recommend you adjust your code to handle this. If you'd really like to disable this, pass `earthaccess.login(..., raise_on_failure=False)`. From cca538f2725fff0cbbbebcdd0f36d845b753104e Mon Sep 17 00:00:00 2001 From: Matt Fisher <3608264+mfisher87@users.noreply.github.com> Date: Mon, 10 Feb 2025 15:59:02 -0700 Subject: [PATCH 11/25] Clarify language in migration guide Co-authored-by: Chuck Daniels --- docs/user_guide/backwards-compatibility.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/user_guide/backwards-compatibility.md b/docs/user_guide/backwards-compatibility.md index fa40ab03..a7b4be4b 100644 --- a/docs/user_guide/backwards-compatibility.md +++ b/docs/user_guide/backwards-compatibility.md @@ -46,5 +46,5 @@ Our project follows the [SPEC0](https://scientific-python.org/specs/spec-0000/) ### 0.14.0 * `earthaccess.login()` will now raise an exception by default if Earthdata Login rejects credentials. - We recommend you adjust your code to handle this. If you'd really like to disable + We recommend you adjust your code to handle this. If you want to disable this, pass `earthaccess.login(..., raise_on_failure=False)`. From 720241daa402ef9c633727996db67ffedf6af48b Mon Sep 17 00:00:00 2001 From: Matt Fisher <3608264+mfisher87@users.noreply.github.com> Date: Mon, 10 Feb 2025 15:59:26 -0700 Subject: [PATCH 12/25] Use explicit python value in docstring Co-authored-by: Chuck Daniels --- earthaccess/api.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/earthaccess/api.py b/earthaccess/api.py index 113ca7ba..734dc3ef 100644 --- a/earthaccess/api.py +++ b/earthaccess/api.py @@ -189,7 +189,7 @@ def login( * **"environment"**: retrieve username and password from `$EARTHDATA_USERNAME` and `$EARTHDATA_PASSWORD`. persist: will persist credentials in a .netrc file system: the Earthdata system to access, defaults to PROD - raise_on_failure: Whether to raise an error when login fails; if false, a warning will be printed + raise_on_failure: Whether to raise an error when login fails; if `False`, only a warning will be printed Returns: An instance of Auth. From f2342311e565b5394d2ab10cc28c3a00cc5f9fde Mon Sep 17 00:00:00 2001 From: Matt Fisher <3608264+mfisher87@users.noreply.github.com> Date: Mon, 10 Feb 2025 16:00:44 -0700 Subject: [PATCH 13/25] Use explicit python value in docstring Co-authored-by: Chuck Daniels --- earthaccess/auth.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/earthaccess/auth.py b/earthaccess/auth.py index 611021af..a24b55b3 100644 --- a/earthaccess/auth.py +++ b/earthaccess/auth.py @@ -115,7 +115,7 @@ def login( Retrieve a username and password from $EARTHDATA_USERNAME and $EARTHDATA_PASSWORD. persist: Will persist credentials in a `.netrc` file. system: the EDL endpoint to log in to Earthdata, defaults to PROD - raise_on_failure: Whether to raise an error when login fails; if false, a warning will be printed + raise_on_failure: Whether to raise an error when login fails; if `False`, only a warning will be printed Returns: An instance of Auth. From bc187813e0e589b206fc43405da17ac0f5dfc577 Mon Sep 17 00:00:00 2001 From: Matt Fisher <3608264+mfisher87@users.noreply.github.com> Date: Mon, 10 Feb 2025 16:04:10 -0700 Subject: [PATCH 14/25] Raise LoginStrategyUnavailable when hostname not found in netrc --- earthaccess/auth.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/earthaccess/auth.py b/earthaccess/auth.py index a24b55b3..216a6523 100644 --- a/earthaccess/auth.py +++ b/earthaccess/auth.py @@ -275,7 +275,9 @@ def _netrc( creds = my_netrc[self.system.edl_hostname] if creds is None: - return False + raise LoginStrategyUnavailable( + f"Earthdata Login hostname {self.system.edl_hostname} not found in .netrc file {netrc_loc}" + ) username = creds["login"] password = creds["password"] From 3154e4733985b6862e243098ac561b2e6fce3f16 Mon Sep 17 00:00:00 2001 From: Matt Fisher <3608264+mfisher87@users.noreply.github.com> Date: Mon, 10 Feb 2025 17:04:55 -0700 Subject: [PATCH 15/25] Add explicit `pass` to exception classes Co-authored-by: Chuck Daniels --- earthaccess/exceptions.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/earthaccess/exceptions.py b/earthaccess/exceptions.py index 452a1a26..731043ca 100644 --- a/earthaccess/exceptions.py +++ b/earthaccess/exceptions.py @@ -7,7 +7,9 @@ class LoginStrategyUnavailable(Exception): DO NOT raise this exception when a login strategy is attempted and failed, for example because credentials were rejected. """ + pass class LoginAttemptFailure(Exception): """The login attempt failed.""" + pass From 2cf8fc63dbb6787247af45f0228c5d1b90afc75f Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Tue, 11 Feb 2025 00:06:55 +0000 Subject: [PATCH 16/25] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- earthaccess/exceptions.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/earthaccess/exceptions.py b/earthaccess/exceptions.py index 731043ca..6283b39c 100644 --- a/earthaccess/exceptions.py +++ b/earthaccess/exceptions.py @@ -7,9 +7,11 @@ class LoginStrategyUnavailable(Exception): DO NOT raise this exception when a login strategy is attempted and failed, for example because credentials were rejected. """ + pass class LoginAttemptFailure(Exception): """The login attempt failed.""" + pass From 5539a2562d3f20e54cee7951a7b4be2628594c38 Mon Sep 17 00:00:00 2001 From: Matt Fisher Date: Mon, 10 Feb 2025 17:18:28 -0700 Subject: [PATCH 17/25] Remove non-user-facing CHANGELOG entry Co-authored-by: Chuck Daniels --- CHANGELOG.md | 7 ------- 1 file changed, 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 38e146eb..3d88d7f1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,13 +22,6 @@ and this project uses [Semantic Versioning](https://semver.org/spec/v2.0.0.html) ## [v0.13.0] - 2025-01-28 -### Changed -- Integration tests: Test are no longer randomized! this means each fail should be - reproducible, we are testing the most popular datasets from all DAACs, see files under - tests/integration/popular_collections. - ([#215](https://github.com/nsidc/earthaccess/issues/215)) - ([**@mfisher87**](https://github.com/mfisher87)) - ### Added - VirtualiZarr: earthaccess can open archival formats (NetCDF, HDF5) as if they were Zarr by leveraging VirtualiZarr In order to use this capability the collection needs to be supported by OPeNDAP and have dmrpp files. From 57535b2b397b852e014b2564395e5f3d6fdb3d2e Mon Sep 17 00:00:00 2001 From: Matt Fisher <3608264+mfisher87@users.noreply.github.com> Date: Mon, 10 Feb 2025 17:21:25 -0700 Subject: [PATCH 18/25] Change voice in changelog entry Co-authored-by: Joseph H Kennedy --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3d88d7f1..9b94f789 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,7 +15,7 @@ and this project uses [Semantic Versioning](https://semver.org/spec/v2.0.0.html) ([**@juliacollins**](https://github.com/juliacollins)) ### Changed -- **Breaking**: Throw an error by default when login credentials are rejected. Pass +- **Breaking**: earthaccess will throw an error by default when login credentials are rejected. Pass `earthaccess.login(..., raise_on_failure=False)` to disable and print a warning instead. ([#946](https://github.com/nsidc/earthaccess/pull/946)) ([**@mfisher87**](https://github.com/mfisher87)) From 1c5de64040195a697cc6e79dcbf25d1080761bea Mon Sep 17 00:00:00 2001 From: Matt Fisher <3608264+mfisher87@users.noreply.github.com> Date: Mon, 10 Feb 2025 17:24:21 -0700 Subject: [PATCH 19/25] More explicit differentiation between exception classes Co-authored-by: Joseph H Kennedy --- earthaccess/exceptions.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/earthaccess/exceptions.py b/earthaccess/exceptions.py index 6283b39c..a3e35046 100644 --- a/earthaccess/exceptions.py +++ b/earthaccess/exceptions.py @@ -4,8 +4,9 @@ class LoginStrategyUnavailable(Exception): This should be raised when a login strategy can't be attempted, for example because "environment" was selected but the envvars are not populated. - DO NOT raise this exception when a login strategy is attempted and failed, for - example because credentials were rejected. + DO NOT raise this exception when a login strategy is attempted and fails. For + example, this exception would not be thrown when credentials were rejected; + a `LoginAttemptFailure` should be thrown instead. """ pass From 6ce736a49abf1e6f015f9fde2aef870d373051a0 Mon Sep 17 00:00:00 2001 From: Matt Fisher <3608264+mfisher87@users.noreply.github.com> Date: Mon, 10 Feb 2025 17:25:16 -0700 Subject: [PATCH 20/25] More explicit differentiation between exception classes Co-authored-by: Joseph H Kennedy --- earthaccess/exceptions.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/earthaccess/exceptions.py b/earthaccess/exceptions.py index a3e35046..fc4fc0e0 100644 --- a/earthaccess/exceptions.py +++ b/earthaccess/exceptions.py @@ -13,6 +13,15 @@ class LoginStrategyUnavailable(Exception): class LoginAttemptFailure(Exception): - """The login attempt failed.""" + """The login attempt failed. + + This should be raised when a login attempt fails, for example, because + the user's credentials were rejected. + + DO NOT raise this exception when a login strategy can't be attempted. For + example, this exception would not be thrown when "environment" was selected + but the envvars are not populated; a `LoginStrategyUnavailable` should be + thrown instead. + """ pass From 3c69ed287f2211b7dbe57fdde869c9680fb25ebe Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Tue, 11 Feb 2025 00:26:01 +0000 Subject: [PATCH 21/25] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- earthaccess/exceptions.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/earthaccess/exceptions.py b/earthaccess/exceptions.py index fc4fc0e0..1108e6f6 100644 --- a/earthaccess/exceptions.py +++ b/earthaccess/exceptions.py @@ -14,10 +14,10 @@ class LoginStrategyUnavailable(Exception): class LoginAttemptFailure(Exception): """The login attempt failed. - + This should be raised when a login attempt fails, for example, because the user's credentials were rejected. - + DO NOT raise this exception when a login strategy can't be attempted. For example, this exception would not be thrown when "environment" was selected but the envvars are not populated; a `LoginStrategyUnavailable` should be From cbac574883228ca4c57a099245e3f4d89e14ad94 Mon Sep 17 00:00:00 2001 From: Matt Fisher Date: Tue, 11 Feb 2025 13:16:10 -0700 Subject: [PATCH 22/25] Remove `raise_on_failure` control parameter --- earthaccess/api.py | 5 ----- earthaccess/auth.py | 51 +++++++++------------------------------------ 2 files changed, 10 insertions(+), 46 deletions(-) diff --git a/earthaccess/api.py b/earthaccess/api.py index 734dc3ef..ab4af71b 100644 --- a/earthaccess/api.py +++ b/earthaccess/api.py @@ -174,8 +174,6 @@ def login( strategy: str = "all", persist: bool = False, system: System = PROD, - *, - raise_on_failure: bool = True, ) -> Auth: """Authenticate with Earthdata login (https://urs.earthdata.nasa.gov/). @@ -189,7 +187,6 @@ def login( * **"environment"**: retrieve username and password from `$EARTHDATA_USERNAME` and `$EARTHDATA_PASSWORD`. persist: will persist credentials in a .netrc file system: the Earthdata system to access, defaults to PROD - raise_on_failure: Whether to raise an error when login fails; if `False`, only a warning will be printed Returns: An instance of Auth. @@ -205,7 +202,6 @@ def login( strategy=strategy, persist=persist, system=system, - raise_on_failure=raise_on_failure, ) except LoginStrategyUnavailable as err: logger.debug(err) @@ -219,7 +215,6 @@ def login( strategy=strategy, persist=persist, system=system, - raise_on_failure=raise_on_failure, ) if earthaccess.__auth__.authenticated: earthaccess.__store__ = Store(earthaccess.__auth__) diff --git a/earthaccess/auth.py b/earthaccess/auth.py index 216a6523..75016f14 100644 --- a/earthaccess/auth.py +++ b/earthaccess/auth.py @@ -100,8 +100,6 @@ def login( strategy: str = "netrc", persist: bool = False, system: Optional[System] = None, - *, - raise_on_failure: bool = True, ) -> Any: """Authenticate with Earthdata login. @@ -115,7 +113,6 @@ def login( Retrieve a username and password from $EARTHDATA_USERNAME and $EARTHDATA_PASSWORD. persist: Will persist credentials in a `.netrc` file. system: the EDL endpoint to log in to Earthdata, defaults to PROD - raise_on_failure: Whether to raise an error when login fails; if `False`, only a warning will be printed Returns: An instance of Auth. @@ -128,11 +125,11 @@ def login( return self if strategy == "interactive": - self._interactive(persist, raise_on_failure=raise_on_failure) + self._interactive(persist) elif strategy == "netrc": - self._netrc(raise_on_failure=raise_on_failure) + self._netrc() elif strategy == "environment": - self._environment(raise_on_failure=raise_on_failure) + self._environment() return self @@ -241,27 +238,17 @@ class Session instance with Auth and bearer token headers def _interactive( self, persist_credentials: bool = False, - *, - raise_on_failure: bool, ) -> bool: username = input("Enter your Earthdata Login username: ") password = getpass.getpass(prompt="Enter your Earthdata password: ") - authenticated = self._get_credentials( - username, - password, - raise_on_failure=raise_on_failure, - ) + authenticated = self._get_credentials(username, password) if authenticated: logger.debug("Using user provided credentials for EDL") if persist_credentials: self._persist_user_credentials(username, password) return authenticated - def _netrc( - self, - *, - raise_on_failure: bool, - ) -> bool: + def _netrc(self) -> bool: netrc_loc = netrc_path() try: @@ -291,22 +278,14 @@ def _netrc( f"Password not found in .netrc file {netrc_loc}" ) - authenticated = self._get_credentials( - username, - password, - raise_on_failure=raise_on_failure, - ) + authenticated = self._get_credentials(username, password) if authenticated: logger.debug("Using .netrc file for EDL") return authenticated - def _environment( - self, - *, - raise_on_failure: bool, - ) -> bool: + def _environment(self) -> bool: username = os.getenv("EARTHDATA_USERNAME") password = os.getenv("EARTHDATA_PASSWORD") @@ -317,30 +296,20 @@ def _environment( ) logger.debug("Using environment variables for EDL") - return self._get_credentials( - username, - password, - raise_on_failure=raise_on_failure, - ) + return self._get_credentials(username, password) def _get_credentials( self, username: Optional[str], password: Optional[str], - *, - raise_on_failure: bool, ) -> bool: if username is not None and password is not None: token_resp = self._find_or_create_token(username, password) if not (token_resp.ok): # type: ignore msg = f"Authentication with Earthdata Login failed with:\n{token_resp.text}" - if raise_on_failure: - logger.error(msg) - raise LoginAttemptFailure(msg) - else: - logger.warning(msg) - return False + logger.error(msg) + raise LoginAttemptFailure(msg) logger.info("You're now authenticated with NASA Earthdata Login") self.username = username From f56742f03715bcc289d66fe2ae183faa5e405c4c Mon Sep 17 00:00:00 2001 From: Matt Fisher Date: Tue, 11 Feb 2025 13:20:44 -0700 Subject: [PATCH 23/25] Update changelog and docs to remove new parameter --- CHANGELOG.md | 9 +++++---- docs/user_guide/backwards-compatibility.md | 12 +++++++++--- 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9b94f789..5e4f7938 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,10 +15,11 @@ and this project uses [Semantic Versioning](https://semver.org/spec/v2.0.0.html) ([**@juliacollins**](https://github.com/juliacollins)) ### Changed -- **Breaking**: earthaccess will throw an error by default when login credentials are rejected. Pass - `earthaccess.login(..., raise_on_failure=False)` to disable and print a warning - instead. ([#946](https://github.com/nsidc/earthaccess/pull/946)) - ([**@mfisher87**](https://github.com/mfisher87)) +- **Breaking**: earthaccess will now raise an exception when login credentials are + rejected. If you need the old behavior, please use a `try` block. + ([#946](https://github.com/nsidc/earthaccess/pull/946)) + ([**@mfisher87**](https://github.com/mfisher87), [@chuckwondo](https://github.com/chuckwondo), + and [@jhkennedy](https://github.com/jhkennedy)) ## [v0.13.0] - 2025-01-28 diff --git a/docs/user_guide/backwards-compatibility.md b/docs/user_guide/backwards-compatibility.md index a7b4be4b..c8bb2b21 100644 --- a/docs/user_guide/backwards-compatibility.md +++ b/docs/user_guide/backwards-compatibility.md @@ -45,6 +45,12 @@ Our project follows the [SPEC0](https://scientific-python.org/specs/spec-0000/) ### 0.14.0 -* `earthaccess.login()` will now raise an exception by default if Earthdata Login rejects credentials. - We recommend you adjust your code to handle this. If you want to disable - this, pass `earthaccess.login(..., raise_on_failure=False)`. +* `earthaccess.login()` will now raise an exception if Earthdata Login rejects credentials. + If you want to ignore errors, which we do not recommend, use a `try` block: + + ```python + try: + earthaccess.login() + except Exception: + pass + ``` From 3e009e670e1f8da9be2e5ec75605c10cc3a22f49 Mon Sep 17 00:00:00 2001 From: Matt Fisher Date: Tue, 11 Feb 2025 13:50:50 -0700 Subject: [PATCH 24/25] Update docstrings with Raises sections --- earthaccess/api.py | 4 ++++ earthaccess/auth.py | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/earthaccess/api.py b/earthaccess/api.py index ab4af71b..e06d2639 100644 --- a/earthaccess/api.py +++ b/earthaccess/api.py @@ -190,6 +190,10 @@ def login( Returns: An instance of Auth. + + Raises: + LoginAttemptFailure: If the NASA Earthdata Login services rejects + credentials. """ # Set the underlying Auth object's earthdata system, # before triggering the getattr function for `__auth__`. diff --git a/earthaccess/auth.py b/earthaccess/auth.py index 75016f14..4308d235 100644 --- a/earthaccess/auth.py +++ b/earthaccess/auth.py @@ -116,6 +116,10 @@ def login( Returns: An instance of Auth. + + Raises: + LoginAttemptFailure: If the NASA Earthdata Login services rejects + credentials. """ if system is not None: self._set_earthdata_system(system) From d0f5d5609a60f8ac5b4ae23c73cc380231a6360d Mon Sep 17 00:00:00 2001 From: Matt Fisher Date: Tue, 11 Feb 2025 13:53:07 -0700 Subject: [PATCH 25/25] Typo --- earthaccess/api.py | 2 +- earthaccess/auth.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/earthaccess/api.py b/earthaccess/api.py index e06d2639..82f62d0d 100644 --- a/earthaccess/api.py +++ b/earthaccess/api.py @@ -192,7 +192,7 @@ def login( An instance of Auth. Raises: - LoginAttemptFailure: If the NASA Earthdata Login services rejects + LoginAttemptFailure: If the NASA Earthdata Login service rejects credentials. """ # Set the underlying Auth object's earthdata system, diff --git a/earthaccess/auth.py b/earthaccess/auth.py index 4308d235..1d1f1162 100644 --- a/earthaccess/auth.py +++ b/earthaccess/auth.py @@ -118,7 +118,7 @@ def login( An instance of Auth. Raises: - LoginAttemptFailure: If the NASA Earthdata Login services rejects + LoginAttemptFailure: If the NASA Earthdata Login service rejects credentials. """ if system is not None: