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

Automatically run earthaccess.login()? #299

Closed
jrbourbeau opened this issue Sep 13, 2023 · 5 comments · Fixed by #300
Closed

Automatically run earthaccess.login()? #299

jrbourbeau opened this issue Sep 13, 2023 · 5 comments · Fixed by #300

Comments

@jrbourbeau
Copy link
Collaborator

I've run into a few times where either I, or a user I was working with, forgot to run earthaccess.login() and get an error like

File "/opt/coiled/env/lib/python3.10/site-packages/earthaccess/api.py", line 184, in open
    results = earthaccess.__store__.open(granules=granules, provider=provider)

AttributeError("'NoneType' object has no attribute 'open'")

I think a better user experience would be to automatically try earthaccess.login() with the netrc and environment strategies. That way, at least for users who have a .netrc file or have intentionally set the right environment variables, earthaccess.login() isn't needed. This removes the "I forgot to run earthaccess.login()" issue and cuts down on boilerplate code (at least by 1 line 🙂).

@betolink @MattF-NSIDC I'm curious what you think about this

@betolink
Copy link
Member

betolink commented Sep 13, 2023

Sounds like a good idea, are you thinking on executing it at import time? I'm planning on working on a PR for this paired with "too verbose" output that you mentioned on #283 @jrbourbeau

@jrbourbeau
Copy link
Collaborator Author

are you thinking on executing it at import time?

My thinking is avoiding a long import time would be nice, as along as the code needed to do things on-demand isn't super complicated

I'm planning on working on a PR for this paired with "too verbose" output that you mentioned on #283

Ah, that'd be great. FWIW I played around a little with the automatic login stuff already. This seems to work for on-demand login:

diff --git a/earthaccess/__init__.py b/earthaccess/__init__.py
index ef4fdfb..dd248b0 100644
--- a/earthaccess/__init__.py
+++ b/earthaccess/__init__.py
@@ -36,7 +36,28 @@ __all__ = [
     "Store",
 ]

-__auth__ = Auth()
-__store__: Any = None
-
 __version__ = version("earthaccess")
+
+_auth = None
+_store = None
+
+def __getattr__(name):
+
+    global _auth, _store
+
+    if name == "__auth__" or name == "__store__":
+        if _auth is None or not _auth.authenticated:
+            for strategy in ["environment", "netrc"]:
+                try:
+                    _auth = Auth().login(strategy=strategy)
+                except Exception:
+                    continue
+                else:
+                    if not _auth.authenticated:
+                        continue
+                    else:
+                        _store = Store(_auth)
+                        break
+        return _auth if name == "__auth__" else _store
+    else:
+        raise AttributeError(f"module {__name__!r} has no attribute {name!r}")

What do you think?

To be clear, that diff ^ doesn't reduce any verbosity during the login process

@MattF-NSIDC
Copy link

❤️ I like this idea. If we don't do automatic login, we should at least make that error message better :)

@betolink
Copy link
Member

Nice! @jrbourbeau if you start a PR with this it would be great, reducing the verbosity should be trivial.

@betolink
Copy link
Member

also, what do you think @JessicaS11? I don't think this should disrupt icepyx

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants