-
Notifications
You must be signed in to change notification settings - Fork 10
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
Download ECCO files using Downloads
and .netrc
files
#281
Merged
Merged
Changes from 1 commit
Commits
Show all changes
33 commits
Select commit
Hold shift + click to select a range
81d93cc
this should work
simone-silvestri 867da9c
better naming
simone-silvestri 71f2b19
only one download
simone-silvestri b36f60f
add download test
simone-silvestri 31fb896
joinpath does not work on windows
simone-silvestri 5b229db
test also downloading the bathymetry
simone-silvestri 3ee4eaa
test dowloading bathymetry
simone-silvestri cba1991
restore tests
simone-silvestri 51aff06
gracefull downloading
simone-silvestri d22edc9
try it now
simone-silvestri ebaa07d
fix typo
simone-silvestri 879d611
make sure we delete the previous data before testing the download
simone-silvestri cd54cdb
Merge branch 'main' into ss/download-everywhere
simone-silvestri 0c38ce1
should work
simone-silvestri 25216bc
Merge branch 'ss/download-everywhere' of github.com:CliMA/ClimaOcean.…
simone-silvestri c0892b9
test distributed downloading
simone-silvestri 6d73e98
Update test_distributed_utils.jl
simone-silvestri 3fc3bd5
fix the download
simone-silvestri f3dec5c
generalize the downloader
simone-silvestri 3525b76
generalize more
simone-silvestri 6a3fa7d
generalize filename
simone-silvestri 91be188
download_progress is part of the downloading utilities
simone-silvestri ab2563d
better docstring
simone-silvestri 25b42cd
better docstring
simone-silvestri a77cebb
change docstring
simone-silvestri 28f597c
Merge branch 'main' into ss/download-everywhere
simone-silvestri c8b6e97
fix tests
simone-silvestri 42da589
Merge branch 'ss/download-everywhere' of github.com:CliMA/ClimaOcean.…
simone-silvestri ad64272
distribute among tasks
simone-silvestri 5c38029
whoops added wrong file
simone-silvestri 926394f
correct looping
simone-silvestri a3254de
bugfix
simone-silvestri 5b65403
Merge branch 'main' into ss/download-everywhere
simone-silvestri File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How does this avoid storing passwords in plain text?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmmm, well it does but then the temporary directory with the netrc file is deleted when the
do tmp; ...; end
block is closed. So in theory for a finite amount of time the password is indeed stored in the filesystem. I can probably rewrite this commentThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In theory? I don't understand. The password is definitely stored in plain text right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not merely "theoretical", the design explicitly stores it in text.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
right, it is definitely stored in plain text
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also is it a problem that
.netrc
is written? Isn't this the recommended way to use.netrc
anyways?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the environment variables route is much cleaner, but it looks like with .netrc we are forced to write down a file. I probably wouldn't like to have a stored file with a password automatically written so I think it is nice to raise this point in the docstring.