Skip to content

Changed return type in download function for consistency. #1031

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Sherwin-14
Copy link
Collaborator

@Sherwin-14 Sherwin-14 commented Jun 11, 2025

I've changed the return type to Path from str as discussed here.


📚 Documentation preview 📚: https://earthaccess--1031.org.readthedocs.build/en/1031/

Copy link

github-actions bot commented Jun 11, 2025

Binder 👈 Launch a binder notebook on this branch for commit 5ec36a3

I will automatically update this comment whenever this PR is modified

Binder 👈 Launch a binder notebook on this branch for commit 5792743

@chuckwondo
Copy link
Collaborator

I reran the integration tests, but they are failing due to CMR internal errors. To confirm that the failures are on the CMR side and not on our side, I tried directly hitting some of the URLs showing up in the integration tests output (e.g., https://cmr.earthdata.nasa.gov/search/granules.umm_json?concept_id%5B%5D=C1692982070-GES_DISC&page_size=0) in a browser. I tried a number of times and I mostly received 500 responses, so I thought perhaps something was wrong with the syntax of the URL itself, but no. I occasionally got a successful response, so it's definitely a problem on the CMR side.

Indeed, during the time I was testing URLs, the NASA Earthdata Status page indicated an issue with CMR.

Therefore, we'll have to rerun the integration tests again later, once the CMR issue is resolved.

@Sherwin-14
Copy link
Collaborator Author

CMR is working now it seems.

@chuckwondo
Copy link
Collaborator

I reran the integration tests 2 more times and they're still failing, but with only 2 failures, so perhaps this is now due to our own test flakiness.

cc: @mfisher87

@mfisher87
Copy link
Collaborator

mfisher87 commented Jun 12, 2025

Nice work, thanks @Sherwin-14 !

If CMR has been down recently, it could be an ongoing issue. Or it could be that the LPDAAC has migrated more data to the cloud. I'm not sure anymore who the best LPDAAC contact point would be 😅 sorry, more and more context is leaving my brain over the months I've been in my new role. @betolink who do you think would know?

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 this pull request may close these issues.

3 participants