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

Login to earthexplorer is only performed once per batch of files #24

Merged
merged 3 commits into from
Aug 14, 2023

Conversation

janosrusiczki
Copy link
Contributor

@janosrusiczki janosrusiczki commented Aug 7, 2023

Moved the earthexplorerLogin() call up the stack so it is only performed once per the batch of files to be downloaded. Additionally made sure the function call is only performed if there are SRTM sources present (wrote a small function to check for that).

Fixes #23

@agrenott
Copy link
Owner

agrenott commented Aug 8, 2023

Thanks for your contribution.
I'm currently off, I'll review it when I'm back.

@janosrusiczki
Copy link
Contributor Author

janosrusiczki commented Aug 8, 2023

@agrenott enjoy your holidays, I can work with my branch until then.

As an additional information I tested with this command and works fine:

pyhgtmap \
  -o contour \
  --step=10 \
  -0 \
  --pbf \
  --hgtdir=/mnt/data/hgt \
  -a $LEFT:$BOTTOM:$RIGHT:$TOP \
  --earthexplorer-user=$EE_USER \
  --earthexplorer-password=$EE_PASSWORD

But I think it should be tested with other sources also.

@agrenott
Copy link
Owner

Looks good considering the overall file's state (NASASRTMUtil would need an overall re-engineering, probably using some API instead of web scraping).
Tested your change rebased on my latest, it works with SRTM1, SRTM3 and view1 sources.

Can you please rebase your PR on the latest master, as this will fix the typing issue?

@janosrusiczki
Copy link
Contributor Author

janosrusiczki commented Aug 14, 2023 via email

@janosrusiczki
Copy link
Contributor Author

Didn't rebase, used the sync fork button on the website which did a merge. Checks are now passing.

@janosrusiczki
Copy link
Contributor Author

Looks good considering the overall file's state (NASASRTMUtil would need an overall re-engineering, probably using some API instead of web scraping).

Let's cross that bridge when we get to it. 😄

I'm working on a set of Docker images to set up an Open Topo Map tile renderer and finding your package was a blessing anyway, the original one being abandoned...

@agrenott agrenott merged commit dd7eed0 into agrenott:master Aug 14, 2023
5 checks passed
@janosrusiczki janosrusiczki deleted the fix-earthexplorer-login-throttle branch August 16, 2023 07:28
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.

The NASA SRTM downloader gets throttled
2 participants