-
Notifications
You must be signed in to change notification settings - Fork 90
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
Change direct access handling to support JupyterHub #880
Conversation
…thods in immediate and deferred test download failure functions
pre-commit.ci autofix |
for more information, see https://pre-commit.ci
I need to add tests for the |
Really happy to see action on #444! Is there a link to any discussion notes leading to the decision to yank Why not include the |
@Alex-Lewandowski, thanks for re-raising this issue. This is definitely a long-standing issue that deserves attention. I believe a thorough solution would be a bit more involved, as there are many issues that have been created around this general problem, so I've just opened a new discussion for this, as I feel it warrants deeper design discussion: #881 Although I appreciate your effort in this PR, I suggest you close it in light of the further discussion it warrants. In the end, the solution in this PR adds more parameters to an already rather cluttered list of parameters for the relevant functions. As part of the discussion, I want to see the parameter list simplified, yet provide more general support for this general issue. |
Hi @itcarroll! There is a bit of discussion on the topic at the bottom of #444. It appears that there is no reliable way to determine a client's region. Boto will provide the default region that the user defined in their configuration, which is not guaranteed to be correct. Most JupyterHubs block access to the EC2 metadata endpoint that earthaccess is currently using to set |
@chuckwondo Okay, it sounds like more discussion is warranted. I will close the PR. |
Closes #444
Currently, earthaccess attempts to determine a client's AWS region by by accessing EC2 metadata at IP 169.254.169.254. However, Zero-to-Jupyterhub-k8s explicitly denies access to the EC2 metadata endpoint for security.
jupyterhub/zero-to-jupyterhub-k8s/jupyterhub/values.yaml
This means that no JupyterHub can currently take advantage of direct access unless it is configured to bypass recommended security settings. Additionally, it fails falsely, informing the user that they are not in
us-west-2
even if they are. There is a workaround, in which you can setin_region
toTrue
before attempting direct access, but you have to know to do this and know to ignore the incorrect out-of-region error.This PR removes the
in_region
member variable and addsout_of_region_handling
andaccess
parameters to the API download function and the store.get functions.Unless
access
is set toexternal
, direct access will be attempted under the assumption that it will succeed. If it fails andout_of_region_handling
is set to "raise" (the default value), the exception is raised. If direct access fails andout_of_region_handling
is set to "handle," a warning is raised, the exception is printed in the warning, and HTTPS is attempted.If 'access' is set to
external
, HTTPS access will be used, and direct access will not be attempted at all.Pull Request (PR) draft checklist - click to expand
contributing documentation
before getting started.
title such as "Add testing details to the contributor section of the README".
Example PRs: #763
example
closes #1
. SeeGitHub docs - Linking a pull request to an issue.
CHANGELOG.md
with details about your change in a section titled## Unreleased
. If such a section does not exist, please create one. FollowCommon Changelog for your additions.
Example PRs: #763
README.md
with details of changes to theearthaccess interface, if any. Consider new environment variables, function names,
decorators, etc.
Click the "Ready for review" button at the bottom of the "Conversation" tab in GitHub
once these requirements are fulfilled. Don't worry if you see any test failures in
GitHub at this point!
Pull Request (PR) merge checklist - click to expand
Please do your best to complete these requirements! If you need help with any of these
requirements, you can ping the
@nsidc/earthaccess-support
team in a comment and wewill help you out!
Request containing "pre-commit.ci autofix" to automate this.
📚 Documentation preview 📚: https://earthaccess--880.org.readthedocs.build/en/880/